feat(core): add VECTOR datatype support with Oracle vector integration#1
Open
prajalg wants to merge 52 commits into
Open
feat(core): add VECTOR datatype support with Oracle vector integration#1prajalg wants to merge 52 commits into
prajalg wants to merge 52 commits into
Conversation
… vector functions
…or distance function
…etc. validate using to only allow hnsw or ivf allowlist distance metrics before emitting WITH DISTANCE require numeric, finite, positive values for accuracy and vector index parameters quote dropConstraintQuery constraint names
…e-dataaccess-sequelize/sequelize-oracle into vector_support_v7
prajalg
changed the title
prajalg
force-pushed
the
vector_support_v7
branch
from
993528c to
484a3d9
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a VECTOR data type, Sequelize vector-distance helpers, and full Oracle support: datatype override with sparse vectors and bind handling, VECTOR_DISTANCE formatting, VECTOR index creation (HNSW/IVF), show-index metadata changes, and extensive unit/integration tests. ChangesCore VECTOR Type and Sequelize Integration
Oracle VECTOR Implementation and Index Support
Comprehensive VECTOR Test Coverage
Sequence Diagram(s)sequenceDiagram
participant App
participant Sequelize
participant OracleQueryGenerator
participant OracleDialect
App->>Sequelize: vectorDistance(column, value)
Sequelize->>OracleDialect: supports?.dataTypes?.VECTOR?
alt supported
OracleDialect-->>Sequelize: true
Sequelize->>OracleQueryGenerator: formatFn(VECTOR_DISTANCE, column, value)
OracleQueryGenerator->>OracleQueryGenerator: normalize value -> VECTOR('[...]')
OracleQueryGenerator-->>Sequelize: fn('VECTOR_DISTANCE', column, literal)
Sequelize-->>App: fn expression
else unsupported
OracleDialect-->>Sequelize: false/undefined
Sequelize-->>App: Error (not implemented)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/test/integration/dialects/oracle/vector.test.js`:
- Around line 404-406: The finally block unconditionally calls
IndexedItem.drop(), which can throw if the table was never created and mask the
real assertion failure; update the cleanup to guard the drop call by checking
existence or swallowing errors: wrap IndexedItem.drop() in a try/catch that
logs/ignores "table not found" errors (or use an API like
IndexedItem.exists()/hasTable() if available) so only genuine failures surface
while ensuring IndexedItem.drop() is still attempted when appropriate.
In `@packages/oracle/src/query-generator.internal.ts`:
- Around line 88-90: Update the VECTOR typed-array handling in `#formatVectorArg`
so it accepts the full set of numeric typed arrays used by core/base (match
packages/oracle/src/_internal/data-types-overrides.ts) instead of only
Float32Array, Float64Array, and Uint8Array: either import and call the existing
isNumericTypedArray (or reference BaseTypes.NumericTypedArray) to decide when to
call `#formatVectorFromIterable`, or expand the instanceof checks to include
Int8Array, Uint8ClampedArray, Int16Array, Uint16Array, Int32Array, and
Uint32Array; ensure the conditional that calls
this.#formatVectorFromIterable(arg) is updated and keep or update the
error/validation message to clearly reflect the supported typed-array set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d2bd3e7f-abb0-4ad9-830c-f8ca459c8c50
📒 Files selected for processing (19)
packages/core/src/abstract-dialect/data-types.tspackages/core/src/abstract-dialect/dialect.tspackages/core/src/abstract-dialect/query-interface.d.tspackages/core/src/data-types.tspackages/core/src/sequelize.d.tspackages/core/src/sequelize.jspackages/core/test/integration/dialects/oracle/vector.test.jspackages/core/test/unit/data-types/string-types.test.tspackages/core/test/unit/dialects/oracle/query-generator.test.jspackages/core/test/unit/dialects/oracle/vector.test.jspackages/core/test/unit/query-generator/show-indexes-query.test.tspackages/core/test/unit/sequelize.test.tspackages/oracle/src/_internal/data-types-overrides.tspackages/oracle/src/dialect.tspackages/oracle/src/query-generator-typescript.internal.tspackages/oracle/src/query-generator.internal.tspackages/oracle/src/query-generator.jspackages/oracle/src/query.jstest/esm-named-exports.test.js
Comment on lines
+88
to
+90
| if (arg instanceof Float32Array || arg instanceof Float64Array || arg instanceof Uint8Array) { | ||
| return this.#formatVectorFromIterable(arg); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Oracle oracledb driver documentation specifies which typed arrays are supported for VECTOR
rg -n "Float32Array|Float64Array|Uint8Array|Int8Array|Int16Array|Int32Array" packages/oracle/src --type ts -C2Repository: prajalg/sequelize
Length of output: 2109
Expand Oracle VECTOR typed-array handling to match core/base numeric typed arrays
packages/oracle/src/query-generator.internal.ts formats only Float32Array, Float64Array, and Uint8Array at lines 88-90, but packages/oracle/src/_internal/data-types-overrides.ts treats BaseTypes.NumericTypedArray as including Int8Array, Uint8ClampedArray, Int16Array, Uint16Array, Int32Array, and Uint32Array as well. This means callers can pass Oracle VECTOR validation and later hit a confusing failure in #formatVectorArg.
Update #formatVectorArg to use the same isNumericTypedArray logic (or expand the instanceof checks to cover the full NumericTypedArray set), or adjust the Oracle-side validation/error message to clearly state which typed arrays are actually supported for VECTOR distance functions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/oracle/src/query-generator.internal.ts` around lines 88 - 90, Update
the VECTOR typed-array handling in `#formatVectorArg` so it accepts the full set
of numeric typed arrays used by core/base (match
packages/oracle/src/_internal/data-types-overrides.ts) instead of only
Float32Array, Float64Array, and Uint8Array: either import and call the existing
isNumericTypedArray (or reference BaseTypes.NumericTypedArray) to decide when to
call `#formatVectorFromIterable`, or expand the instanceof checks to include
Int8Array, Uint8ClampedArray, Int16Array, Uint16Array, Int32Array, and
Uint32Array; ensure the conditional that calls
this.#formatVectorFromIterable(arg) is updated and keep or update the
error/validation message to clearly reflect the supported typed-array set.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Description of Changes
This PR adds
VECTORsupport with Oracle-first implementation, shared core abstractions, and full validation/test coverage for vector SQL generation paths.Core datatype & API groundwork
AbstractVECTORBase(shared behavior/validation)DataTypes.VECTORconstructor overloads:DataTypes.VECTORDataTypes.VECTOR(dimension)DataTypes.VECTOR(dimension, format)DataTypes.VECTOR({ dimension, format })cosineDistanceinnerProductl1Distancel2DistancevectorDistanceOracle integration
supports.dataTypes.VECTOR = true.SPARSE) and bind marshalling.usingdistanceaccuracyparametersorderTests
Guidance for other dialects
For future dialect integration, this PR establishes the recommended path:
supports.dataTypes.VECTOR.VECTORwhere generic option shape is sufficient, or extendAbstractVECTORBasefor dialect-specific option models.List of Breaking Changes
Summary by CodeRabbit
New Features
Enhancements
Tests