feat(sap): add support for REAL_VECTOR and HALF_VECTOR data types in SAP HANA Cloud #11526
Conversation
WalkthroughSupport for SAP HANA vector column types ( Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant TypeORM as TypeORM
participant SapDriver as SAP Driver
participant SAPDB as SAP HANA DB
App->>TypeORM: Define entity with real_vector/half_vector columns
TypeORM->>SapDriver: Prepare schema (synchronize)
SapDriver->>SAPDB: Create table with vector columns
App->>TypeORM: Save entity with vector data
TypeORM->>SapDriver: Prepare persistent value (array/buffer)
SapDriver->>SAPDB: INSERT with vector data
App->>TypeORM: Load entity
TypeORM->>SapDriver: Hydrate value (array/buffer)
SapDriver->>SAPDB: SELECT vector columns
SAPDB-->>SapDriver: Return vector data (Buffer)
SapDriver-->>TypeORM: Convert to array (if configured)
TypeORM-->>App: Return hydrated entity
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
docs/docs/entity/1-entities.md (1)
406-408: Remove blank line inside blockquote
Eliminate the empty line between the two>lines to comply with markdownlint (MD028).@@ -407,1 +407,0 -🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
407-407: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
test/functional/database-schema/column-types/sap/entity/Embedding.ts (2)
5-6: Consider using@PrimaryGeneratedColumninstead of manual IDsUnless you intentionally need to supply the
idyourself, using@PrimaryGeneratedColumn()avoids manual bookkeeping and prevents accidental PK collisions.
11-13:metadatalooks like JSON – leveragesimple-jsonIf the column will always hold JSON, declaring
@Column("simple-json")gives automatic (de)serialization and saves future consumers from string parsing.test/functional/database-schema/column-types/sap/column-types-sap.ts (1)
356-368: Use.includefor partial-object assertionsEarlier in this file you rely on
expect(obj).to.include({...}); here you switched to.contain. While Chai aliases the two, sticking to one style improves readability and avoids confusion for new contributors.src/driver/sap/SapDriver.ts (1)
175-176: Why allow scale but not precision fortimestamp?
TIMESTAMP(p)in SAP HANA supports precision; you added it only towithScaleColumnTypes. Consider adding it towithPrecisionColumnTypesfor completeness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/docs/entity/1-entities.md(1 hunks)src/driver/sap/SapDriver.ts(6 hunks)src/driver/types/ColumnTypes.ts(2 hunks)test/functional/database-schema/column-types/sap/column-types-sap.ts(2 hunks)test/functional/database-schema/column-types/sap/entity/Embedding.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/Post.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/PostWithOptions.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/PostWithoutTypes.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/driver/sap/SapDriver.ts (1)
src/driver/types/ColumnTypes.ts (1)
ColumnType(216-225)
🪛 LanguageTool
docs/docs/entity/1-entities.md
[uncategorized] ~404-~404: Loose punctuation mark.
Context: ... ### Column types forsap tinyint, smallint, integer, bigint, small...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/entity/1-entities.md
407-407: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: database-tests-compose (18) / sap
- GitHub Check: database-tests (20) / mssql (^10.0.1)
- GitHub Check: database-tests (20) / mssql (^9.1.1)
- GitHub Check: database-tests (20) / mssql (^11.0.1)
- GitHub Check: database-tests (18) / sqlite
- GitHub Check: database-tests (20) / mongodb
- GitHub Check: database-tests-windows / sqljs
- GitHub Check: database-tests (18) / mssql (^9.1.1)
- GitHub Check: database-tests (18) / sqljs
- GitHub Check: database-tests (18) / better-sqlite3
- GitHub Check: database-tests (18) / mssql (^11.0.1)
- GitHub Check: database-tests (18) / mssql (^10.0.1)
- GitHub Check: database-tests (18) / mongodb
- GitHub Check: database-tests (18) / cockroachdb
- GitHub Check: database-tests (18) / mysql_mariadb
- GitHub Check: database-tests (18) / postgres (17-3.5)
- GitHub Check: database-tests (18) / postgres (14-3.5)
- GitHub Check: database-tests-windows / sqlite
- GitHub Check: database-tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
test/functional/database-schema/column-types/sap/entity/PostWithoutTypes.ts (1)
1-1: Consolidate imports for consistency
Combined the decorator imports into a single line, matching the style used across other SAP entity tests.test/functional/database-schema/column-types/sap/entity/PostWithOptions.ts (1)
1-1: Consolidated imports for consistency
MergedEntity,PrimaryColumn, andColumnimports into one line to align with the import style in related test files.src/driver/types/ColumnTypes.ts (2)
49-51: Extend WithPrecisionColumnType withtimestamp
The addition of"timestamp"correctly enables precision-based timestamp support across SAP and other platforms.
78-78: Addreal_vectorto WithLengthColumnType
Introduces the SAP-specificreal_vectortype into the length-based union, matching driver and documentation updates.test/functional/database-schema/column-types/sap/entity/Post.ts (1)
1-1: Consolidate imports for consistency
Unified the decorator imports into a single line, consistent with other SAP entity test files.docs/docs/entity/1-entities.md (1)
404-404: Includereal_vectorin SAP column types list
Adds the newreal_vectordata type to the SAP column types enumeration, reflecting the newly supported vector type.🧰 Tools
🪛 LanguageTool
[uncategorized] ~404-~404: Loose punctuation mark.
Context: ...### Column types forsaptinyint,smallint,integer,bigint,small...(UNLIKELY_OPENING_PUNCTUATION)
src/driver/sap/SapDriver.ts (2)
494-513: Removed Boolean coercion – double-check backward compatibilityOld code converted booleans to
1/0; now rawtrue/falseis sent. Ensure all supported SAP HANA versions accept literal booleans in INSERT/UPDATE, otherwise migrations on older DBs may break.
614-619: Good fallback forreal_vector, but document itMapping
real_vector➜blobon (< 4.0) clusters is sound; add a comment or doc entry so users understand why queries showblobafter sync.
|
Yes, I used I will remove the fallback since I cannot find another alternative and dynamically register the entity. |
bb39eb2 to
ecd9b4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/docs/entity/1-entities.md (1)
406-409: Remove the blank line inside the quoted note (MD028)A blank line immediately after the opening
>breaks the block-quote and is flagged bymarkdownlint.-> Note 2: SAP HANA Cloud introduced the `real_vector` data type … - > Note 2: SAP HANA Cloud introduced the `real_vector` data type …test/functional/database-schema/vector-columns/sap/sap-real-vector.ts (2)
13-14: Drop theasynckeyword fromdescribecallbacksMocha ignores promises returned by
describebodies; marking themasyncis misleading and may surface unhandled-rejection warnings in future Mocha versions.-describe("real_vector with output type Array", async () => { +describe("real_vector with output type Array", () => { … -describe("real_vector with output type Buffer", async () => { +describe("real_vector with output type Buffer", () => {Also applies to: 107-108
118-144: Minor: preferFloat32Arrayfor (de)serialising vectorsUsing typed arrays avoids the explicit byte-loop and is easier to read:
-function deserializeFvecs(buffer: Buffer) { - const dataView = new DataView( - buffer.buffer, - buffer.byteOffset, - buffer.byteLength, - ) - const length = dataView.getUint32(0, true) - const array = new Array<number>(length) - for (let index = 0; index < length; index++) { - array[index] = dataView.getFloat32(4 + index * 4, true) - } - return array -} +function deserializeFvecs(buffer: Buffer) { + const view = new DataView(buffer.buffer, buffer.byteOffset) + const length = view.getUint32(0, true) + return Array.from( + new Float32Array(buffer.buffer, buffer.byteOffset + 4, length), + ) +} … -function serializeFvecs(array: number[]) { - const length = array.length - const arrayBuffer = new ArrayBuffer(4 + length * 4) - const dataView = new DataView(arrayBuffer) - dataView.setUint32(0, length, true) - for (let index = 0; index < length; index++) { - dataView.setFloat32(4 + index * 4, array[index], true) - } - return Buffer.from(arrayBuffer) -} +function serializeFvecs(values: number[]) { + const buffer = Buffer.alloc(4 + values.length * 4) + buffer.writeUInt32LE(values.length, 0) + new Float32Array(buffer.buffer, buffer.byteOffset + 4, values.length).set( + values, + ) + return buffer +}Purely a readability / micro-perf tweak – feel free to ignore.
Also applies to: 133-144
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/docs/entity/1-entities.md(1 hunks)src/driver/sap/SapDriver.ts(6 hunks)src/driver/types/ColumnTypes.ts(2 hunks)src/metadata-builder/EntityMetadataValidator.ts(1 hunks)test/functional/database-schema/column-types/sap/column-types-sap.ts(2 hunks)test/functional/database-schema/column-types/sap/entity/Post.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/PostWithOptions.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/PostWithoutTypes.ts(1 hunks)test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/entity/FvecsEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/sap-real-vector.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/functional/database-schema/column-types/sap/entity/PostWithoutTypes.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- test/functional/database-schema/column-types/sap/entity/PostWithOptions.ts
- src/metadata-builder/EntityMetadataValidator.ts
- src/driver/types/ColumnTypes.ts
- test/functional/database-schema/column-types/sap/entity/Post.ts
- test/functional/database-schema/column-types/sap/column-types-sap.ts
- src/driver/sap/SapDriver.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
test/functional/database-schema/vector-columns/sap/entity/FvecsEmbedding.ts (3)
test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts (1)
Entity(3-27)src/decorator/columns/PrimaryColumn.ts (1)
PrimaryColumn(37-100)src/decorator/columns/Column.ts (1)
Column(134-220)
test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts (3)
test/functional/database-schema/vector-columns/sap/entity/FvecsEmbedding.ts (1)
Entity(3-16)src/decorator/columns/PrimaryColumn.ts (1)
PrimaryColumn(37-100)src/decorator/columns/Column.ts (1)
Column(134-220)
test/functional/database-schema/vector-columns/sap/sap-real-vector.ts (3)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)reloadTestingDatabases(500-505)closeTestingConnections(487-495)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)src/common/DeepPartial.ts (1)
DeepPartial(4-14)
🪛 LanguageTool
docs/docs/entity/1-entities.md
[uncategorized] ~404-~404: Loose punctuation mark.
Context: ... ### Column types forsap tinyint, smallint, integer, bigint, small...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/entity/1-entities.md
407-407: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: tests-linux (20) / sqljs
- GitHub Check: tests-linux (20) / sqlite
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: tests-linux (18) / postgres (14-3.5)
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / postgres (17-3.5)
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
docs/docs/entity/1-entities.md (1)
404-404:real_vectoraddition looks correctThe new data-type is appended consistently with the existing, comma-separated list and ends the sentence with a full stop – no functional or stylistic issues here.
test/functional/database-schema/vector-columns/sap/entity/FvecsEmbedding.ts (1)
1-16: Entity definition is minimal and correctPrimary key, text columns and the binary
real_vectorcolumn are declared properly;synchronize: falsematches other schema tests.test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts (1)
14-27: Well-structured column samplingSmall, large (nullable) and variable-length vectors give the test suite good coverage. Implementation LGTM.
ecd9b4b to
9237a57
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/docs/entity/1-entities.md (1)
409-409: Fix duplicated word and gerund usage
Remove the repeated “the” and switch to a gerund for smoother phrasing.- TypeORM's `SapDriver` supports `tinyint`, `smallint`, `integer`, `bigint`, `smalldecimal`, `decimal`, `real`, `double`, `date`, `time`, `seconddate`, `timestamp`, `boolean`, `char`, `nchar`, `varchar`, `nvarchar`, `text`, `alphanum`, `shorttext`, `array`, `varbinary`, `blob`, `clob`, `nclob`, `st_geometry`, `st_point`, `real_vector`. Some of these are deprecated or removed in SAP HANA Cloud, and will be converted to the closest available alternative when connected to a Cloud database. We strongly suggest to use the data types native the the version of HANA you are using. + TypeORM's `SapDriver` supports `tinyint`, `smallint`, `integer`, `bigint`, `smalldecimal`, `decimal`, `real`, `double`, `date`, `time`, `seconddate`, `timestamp`, `boolean`, `char`, `nchar`, `varchar`, `nvarchar`, `text`, `alphanum`, `shorttext`, `array`, `varbinary`, `blob`, `clob`, `nclob`, `st_geometry`, `st_point`, `real_vector`. Some of these types are deprecated or removed in SAP HANA Cloud and will be converted to the closest available alternative when connected to a Cloud database. We strongly suggest using data types native to the version of HANA you are using.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/docs/entity/1-entities.md(1 hunks)src/driver/sap/SapDriver.ts(6 hunks)test/functional/database-schema/column-types/sap/column-types-sap.ts(2 hunks)test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/entity/FvecsEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/sap-real-vector.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- test/functional/database-schema/vector-columns/sap/entity/FvecsEmbedding.ts
- test/functional/database-schema/column-types/sap/column-types-sap.ts
- test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts
- test/functional/database-schema/vector-columns/sap/sap-real-vector.ts
- src/driver/sap/SapDriver.ts
🧰 Additional context used
🪛 LanguageTool
docs/docs/entity/1-entities.md
[grammar] ~409-~409: The verb ‘suggest’ is used with the gerund form.
Context: ...nected to a Cloud database. We strongly suggest to use the data types native the the version o...
(ADMIT_ENJOY_VB)
[duplication] ~409-~409: Possible typo: you repeated a word.
Context: ...ly suggest to use the data types native the the version of HANA you are using. The `re...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~411-~411: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ta type was introduced in SAP HANA Cloud and it is usable since `@sap/hana-client@2....
(COMMA_COMPOUND_SENTENCE)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: tests-linux (20) / sap
- GitHub Check: tests-linux (20) / sqljs
- GitHub Check: tests-linux (20) / postgres (17-3.5)
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (20) / better-sqlite3
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (18) / postgres (14-3.5)
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / postgres (17-3.5)
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
docs/docs/entity/1-entities.md (1)
404-408: Great addition of SAP HANA 2.0 and Cloud links.
The newly added references to the official SAP Help pages will guide users to the authoritative source for data type differences.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/docs/entity/1-entities.md (1)
409-412: Still missing thelengthoption example & minor grammar nitThe earlier review already asked to document how to fix the vector length:
@Column("real_vector", { length: 1024 }) embeddings: number[];That snippet is still absent. Also, add a comma before “and”:
“…introduced in SAP HANA Cloud, and it is usable…”
Please incorporate the example to make the feature discoverable.
🧹 Nitpick comments (2)
package.json (1)
239-239:packageManagerlock-in: verify CI & release toolingLocking the repo to
pnpm@9.15.9is fine, but ensure:
- All CI jobs and release pipelines already use this exact pnpm version (including SHA).
- Team onboarding docs mention the change.
corepack enableis run in pre-install hooks for reproducibility.Otherwise builds may break on older Node setups.
test/functional/database-schema/vector-columns/sap/sap-vector.ts (1)
51-65:variableVectorlength assertion is driver-specific
Column.lengthmay benull,undefined, or""depending on dialect implementation.
Hard-coding""will make the test brittle across driver versions.-expect(table.findColumnByName("variableVector")).to.contain({ - type: "real_vector", - length: "", -}) +const variableCol = table.findColumnByName("variableVector")! +expect(variableCol.type).to.equal("real_vector") +expect(variableCol.length ?? "").to.equal("")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
docs/docs/entity/1-entities.md(1 hunks)package.json(2 hunks)src/driver/sap/SapDriver.ts(6 hunks)src/driver/types/ColumnTypes.ts(2 hunks)test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/entity/BufferEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/sap-vector.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/driver/types/ColumnTypes.ts
- test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts
- src/driver/sap/SapDriver.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/functional/database-schema/vector-columns/sap/sap-vector.ts (3)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)reloadTestingDatabases(500-505)closeTestingConnections(487-495)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)src/common/DeepPartial.ts (1)
DeepPartial(4-14)
🪛 LanguageTool
docs/docs/entity/1-entities.md
[uncategorized] ~411-~411: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ta type was introduced in SAP HANA Cloud and it is usable since `@sap/hana-client@2....
(COMMA_COMPOUND_SENTENCE)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / better-sqlite3
- GitHub Check: tests-linux (20) / sqlite
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / postgres (14-3.5)
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / postgres (14-3.5)
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
package.json (1)
109-113: Align peer-dependency floor with the newly required client version
@sap/hana-clientis bumped to^2.25.22in devDependencies, but thepeerDependenciesfloor is still^2.12.25.
All functional tests introduced here (vector types, esp.half_vector) require>= 2.25.x. Leaving the lower range inpeerDependencieswill let downstream projects install a version that does not support these types and fail at runtime.- "@sap/hana-client": "^2.12.25", + "@sap/hana-client": "^2.25.22",Please update the peer range – or, if full backward-compatibility is intended, guard the new driver logic with explicit version checks.
test/functional/database-schema/vector-columns/sap/entity/BufferEmbedding.ts (1)
1-16: Looks good – minimal, readable entity definitionThe entity maps the new
real_vectortype correctly and keeps tests focused.test/functional/database-schema/vector-columns/sap/sap-vector.ts (2)
112-120:synchronizeflag is misplaced
driverSpecific.synchronizeis ignored bycreateTestingConnections; the flag belongs
at the root of the DataSource options. This test happens to callsynchronize()later,
but the intention is unclear. Move it one level up for clarity.
125-151: Helper (de)serialisers – good & self-containedThe FVEC helpers correctly marshal length-prefixed
float32arrays and keep the test
independent from external libs.
1248086 to
b0d5a00
Compare
b0d5a00 to
2998ced
Compare
2998ced to
512c365
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/docs/entity/1-entities.md (1)
411-412: Document thelengthoption for vector types.Users should know they can specify a fixed vector size. Please add an example showing the
{ length: 1024 }option in the@Column("real_vector", { length: 1024 })decorator, as previously suggested.
🧹 Nitpick comments (1)
docs/docs/entity/1-entities.md (1)
411-411: Fix missing preposition.Insert "of" after "versions" for grammatical correctness:
- The `real_vector` and `half_vector` data types were introduced in recent versions SAP HANA Cloud (2024Q1 and 2025Q2 respectively)... + The `real_vector` and `half_vector` data types were introduced in recent versions of SAP HANA Cloud (2024Q1 and 2025Q2 respectively)...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
docs/docs/entity/1-entities.md(1 hunks)package.json(1 hunks)src/driver/sap/SapDriver.ts(7 hunks)src/driver/types/ColumnTypes.ts(2 hunks)src/metadata-builder/EntityMetadataValidator.ts(1 hunks)test/functional/database-schema/column-types/sap/column-types-sap.ts(2 hunks)test/functional/database-schema/column-types/sap/entity/Post.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/PostWithOptions.ts(1 hunks)test/functional/database-schema/column-types/sap/entity/PostWithoutTypes.ts(1 hunks)test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/entity/BufferEmbedding.ts(1 hunks)test/functional/database-schema/vector-columns/sap/sap-vector.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/functional/database-schema/column-types/sap/entity/PostWithoutTypes.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- test/functional/database-schema/column-types/sap/entity/PostWithOptions.ts
- package.json
- src/metadata-builder/EntityMetadataValidator.ts
- test/functional/database-schema/vector-columns/sap/entity/BufferEmbedding.ts
- src/driver/types/ColumnTypes.ts
- test/functional/database-schema/column-types/sap/entity/Post.ts
- test/functional/database-schema/column-types/sap/column-types-sap.ts
- test/functional/database-schema/vector-columns/sap/sap-vector.ts
- test/functional/database-schema/vector-columns/sap/entity/ArrayEmbedding.ts
- src/driver/sap/SapDriver.ts
🧰 Additional context used
🪛 LanguageTool
docs/docs/entity/1-entities.md
[uncategorized] ~411-~411: Possible missing preposition found.
Context: ...ypes were introduced in recent versions SAP HANA Cloud (2024Q1 and 2025Q2 respectiv...
(AI_HYDRA_LEO_MISSING_OF)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: tests-linux (20) / postgres (14-3.5)
- GitHub Check: tests-linux (20) / sqlite
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (20) / sqljs
- GitHub Check: tests-linux (20) / sap
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / postgres (17-3.5)
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
docs/docs/entity/1-entities.md (2)
404-408: Approve documentation updates.The section introducing SAP HANA 2.0 vs Cloud data types and the links are clear and accurate.
409-410: Approve supported data types list.Inclusion of
real_vectorandhalf_vectorin theSapDriverlist is correct and aligns with the feature implementation.
512c365 to
e98394f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/functional/database-schema/vectors/sap/sap-vector.ts (1)
31-38: Consider adding error handling for unsupported driver versions.While the driver version check correctly skips tests for versions < 4.0, consider adding a console warning or test skip message to make it clear why tests are being skipped in CI environments.
if ( !DriverUtils.isReleaseVersionOrGreater( dataSource.driver, "4.0", ) ) { + console.warn("Skipping SAP vector tests - driver version < 4.0 required") return }Also applies to: 153-160
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
docs/docs/entity/1-entities.md(1 hunks)package.json(1 hunks)src/driver/sap/SapDriver.ts(7 hunks)src/driver/types/ColumnTypes.ts(2 hunks)test/functional/database-schema/column-types/sap/column-types-sap.ts(2 hunks)test/functional/database-schema/vectors/sap/entity/ArrayEmbedding.ts(1 hunks)test/functional/database-schema/vectors/sap/entity/BufferEmbedding.ts(1 hunks)test/functional/database-schema/vectors/sap/sap-vector.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/functional/database-schema/vectors/sap/entity/ArrayEmbedding.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- package.json
- src/driver/types/ColumnTypes.ts
- test/functional/database-schema/column-types/sap/column-types-sap.ts
- docs/docs/entity/1-entities.md
- src/driver/sap/SapDriver.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/functional/database-schema/vectors/sap/entity/BufferEmbedding.ts (3)
test/functional/database-schema/vectors/sap/entity/ArrayEmbedding.ts (1)
Entity(3-27)src/decorator/columns/PrimaryColumn.ts (1)
PrimaryColumn(37-100)src/decorator/columns/Column.ts (1)
Column(134-220)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: tests-linux (18) / postgres (17-3.5)
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (20) / oracle
- GitHub Check: tests-linux (20) / postgres (14-3.5)
- GitHub Check: tests-linux (20) / postgres (17-3.5)
- GitHub Check: tests-linux (20) / better-sqlite3
- GitHub Check: tests-linux (20) / sqlite
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / sqljs
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (20) / sap
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
test/functional/database-schema/vectors/sap/entity/BufferEmbedding.ts (1)
1-16: LGTM! Clean entity definition for SAP vector support.The
BufferEmbeddingentity is well-structured and correctly implements the newreal_vectorcolumn type for SAP HANA Cloud. The use ofBuffertype for the vector column aligns with the binary storage format, and the variable-length vector approach (no length specification) provides flexibility for different vector sizes.test/functional/database-schema/vectors/sap/sap-vector.ts (4)
11-26: Good test structure with proper driver version checks.The test setup correctly configures the SAP driver with
vectorOutputType: "Array"and disables synchronization by default, which is appropriate for functional testing.
28-106: Comprehensive schema validation and data persistence testing.The Array output type test thoroughly validates:
- Schema synchronization and column metadata verification
- Data persistence and hydration with realistic vector data
- Proper handling of different vector configurations (fixed-length, nullable, variable-length)
The test data uses realistic floating-point vectors and properly validates the round-trip persistence.
150-196: Buffer output type test covers binary vector persistence well.The Buffer output type test properly:
- Uses the default vector output configuration (Buffer)
- Tests serialization to fvecs format before persistence
- Validates deserialization after hydration
- Uses the same realistic test data for consistency
The test logic correctly handles the Buffer ↔ number[] conversion cycle.
122-148: Verify fvecs format implementation for correctness.The fvecs serialization/deserialization functions appear to implement the correct binary format (length-prefixed float32 array with little-endian byte order). However, please verify this matches the exact SAP HANA Client fvecs specification.
What is the exact binary format specification for fvecs (float vector) format used by SAP HANA Client?
| if (columnMetadata.type === Boolean) { | ||
| return value === true ? 1 : 0 |
There was a problem hiding this comment.
Is it intentional to lose the boolean conversions here?
There was a problem hiding this comment.
I think it was copy-pasted from a driver that doesn't support boolean (mssql?)
But @sap/hana-client is okay with receiving boolean values, no explicit conversion to 0 & 1 is needed.
…SAP HANA Cloud (typeorm#11526) * feat(sap): add support for REAL_VECTOR data type * feat(sap): add support for HALF_VECTOR data type
Description of change
Add support for
REAL_VECTORandHALF_VECTORin SAP HANA Cloud. Optional parameter:length, for a fixed-length vector.Usage:
or, with the connection option
vectorOutputType=Array:Further Documentation:
Pull-Request Checklist
masterbranchFixes #00000Summary by CodeRabbit
real_vectorandhalf_vector, including schema and persistence capabilities for both array and buffer representations.@sap/hana-clientdependency to the latest version.