-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Oraclevs integration #8007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Oraclevs integration #8007
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
@cjbj can you please let us know if you have any more comments. We would be really grateful if we can merge this in. Several of our customers with a large JS presence would really like to use it as Langchain is the go to platform for everyone nowadays and Oracle DB is where all of their mission critical data is. |
docs/core_docs/docs/integrations/document_loaders/file_loaders/oracleai.mdx
Outdated
Show resolved
Hide resolved
docs/core_docs/docs/integrations/document_loaders/file_loaders/oracleai.mdx
Outdated
Show resolved
Hide resolved
docs/core_docs/docs/integrations/document_loaders/file_loaders/oracleai.mdx
Outdated
Show resolved
Hide resolved
} | ||
const result = await this.conn.execute( | ||
<string>( | ||
`select dbms_vector_chain.utl_to_text(t.${this.pref.colname}, :pref) text, dbms_vector_chain.utl_to_text(t.${this.pref.colname}, json('{"plaintext": "false"}')) metadata from ${this.pref.owner}.${this.pref.tablename} t` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have the substituted values been filtered to avoid any SQL Injection issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have added a check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You need to bind those values !
- Would it be nice to throw a custom error message if the check fails?
const cn = 'empno';
const ow = 'scott';
const tn = 'emp';
const qn = `${ow}.${tn}`;
const sql = `select sys.dbms_assert.simple_sql_name(:sn), sys.dbms_assert.qualified_sql_name(:qn) from dual`;
const binds = [cn, qn];
await connection.execute(sql, binds);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to bind values and added a custom error message as suggested
docs/core_docs/docs/integrations/document_loaders/file_loaders/oracleai.mdx
Outdated
Show resolved
Hide resolved
libs/langchain-community/src/vectorstores/tests/oraclevs_test.ts
Outdated
Show resolved
Hide resolved
const embeddings = []; | ||
|
||
if (oracledb.thin) { | ||
// thin mode, can't use batching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is something missing in node-oracledb Thin mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The batching version of utl_to_embedding passes a collection called vector_array_t which is an array of clobs and I think thin mode didn't support this. Hence, in thin mode we embed each input at a time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. We (node-oracledb) will revisit this.
Description
This transaction integrates Oracle Vector Search with LangChain.js. The integration enables the use of Oracle's advanced vector search capabilities within the LangChain.js framework.
Key Features
Adds support for Oracle Vector Search in LangChain.js.
Includes example usage and tests for the integration.
Fixes
Testing
Verified integration with unit tests.
Added relevant test cases to ensure proper functionality.
Request
Please let @skmishraoracle and @rohanaggarwal7997 know if there are any issues or areas that need improvement.