mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(Databricks Node): Guard the Vector Store insert against the wrong index (#39699)
This commit is contained in:
parent
830486a851
commit
0932e9572e
|
|
@ -460,16 +460,38 @@ describe('DatabricksVectorStore', () => {
|
|||
).rejects.toThrow('Failed primary keys: row-7');
|
||||
});
|
||||
|
||||
it('rejects the primary key as the content column without a request', async () => {
|
||||
const store = await createStore(directDescribe, { contentColumn: 'id' });
|
||||
it.each([
|
||||
['txet', 'Index cat.sch.idx has no column txet. Select a Content Column from: text, source'],
|
||||
['id', 'Column id is the primary key of cat.sch.idx. Select another Content Column'],
|
||||
[
|
||||
'embedding',
|
||||
'Column embedding holds the embedding vector of cat.sch.idx. Select another Content Column',
|
||||
],
|
||||
])('rejects %s as the content column before embedding anything', async (column, message) => {
|
||||
const store = await createStore(directDescribe, { contentColumn: column });
|
||||
|
||||
await expect(store.addDocuments([{ pageContent: 'hello', metadata: {} }])).rejects.toThrow(
|
||||
'primary key',
|
||||
message,
|
||||
);
|
||||
expect(fetchMock).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining('upsert-data'),
|
||||
expect.anything(),
|
||||
expect(embeddings.embedDocuments).not.toHaveBeenCalled();
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects an index that declares an empty schema, rather than letting Databricks reject it', async () => {
|
||||
const emptySchema = {
|
||||
...directDescribe,
|
||||
direct_access_index_spec: {
|
||||
...directDescribe.direct_access_index_spec,
|
||||
schema_json: '{}',
|
||||
},
|
||||
};
|
||||
const store = await createStore(emptySchema, { contentColumn: 'text' });
|
||||
|
||||
await expect(store.addDocuments([{ pageContent: 'hello', metadata: {} }])).rejects.toThrow(
|
||||
'Index cat.sch.idx has no column text, and declares no other column to select',
|
||||
);
|
||||
expect(embeddings.embedDocuments).not.toHaveBeenCalled();
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a managed Delta Sync index without a request', async () => {
|
||||
|
|
|
|||
|
|
@ -299,7 +299,7 @@ export class DatabricksVectorStore extends VectorStore {
|
|||
options?: { ids?: string[] },
|
||||
): Promise<string[]> {
|
||||
if (documents.length === 0) return [];
|
||||
this.assertDirectAccess();
|
||||
this.assertWritableContentColumn();
|
||||
const vectors = await this.embeddings.embedDocuments(documents.map((doc) => doc.pageContent));
|
||||
return await this.addVectors(vectors, documents, options);
|
||||
}
|
||||
|
|
@ -309,14 +309,8 @@ export class DatabricksVectorStore extends VectorStore {
|
|||
documents: DocumentInterface[],
|
||||
options?: { ids?: string[] },
|
||||
): Promise<string[]> {
|
||||
const vectorColumn = this.assertDirectAccess();
|
||||
const vectorColumn = this.assertWritableContentColumn();
|
||||
const { primaryKey, name } = this.index;
|
||||
// The row below would write the document text over its own primary key
|
||||
if (this.contentColumn === primaryKey) {
|
||||
throw new UserError(
|
||||
`Column ${primaryKey} is the primary key of ${name}. Select another content column`,
|
||||
);
|
||||
}
|
||||
const schemaColumns = new Set(this.index.schemaColumns ?? []);
|
||||
const ids = documents.map((doc, i) => options?.ids?.[i] ?? doc.id ?? randomUUID());
|
||||
|
||||
|
|
@ -347,6 +341,33 @@ export class DatabricksVectorStore extends VectorStore {
|
|||
return ids;
|
||||
}
|
||||
|
||||
/** Both writes below would land on a column the row already fills from somewhere else. */
|
||||
private assertWritableContentColumn(): string {
|
||||
const vectorColumn = this.assertDirectAccess();
|
||||
const { primaryKey, name, schemaColumns } = this.index;
|
||||
if (this.contentColumn === primaryKey) {
|
||||
throw new UserError(
|
||||
`Column ${primaryKey} is the primary key of ${name}. Select another Content Column`,
|
||||
);
|
||||
}
|
||||
if (this.contentColumn === vectorColumn) {
|
||||
throw new UserError(
|
||||
`Column ${vectorColumn} holds the embedding vector of ${name}. Select another Content Column`,
|
||||
);
|
||||
}
|
||||
// An index that declares an empty schema is known, not unknown: letting it
|
||||
// through trades this message for a raw Databricks schema error.
|
||||
if (schemaColumns !== undefined && !schemaColumns.includes(this.contentColumn)) {
|
||||
const selectable = schemaColumns.filter((column) => !this.reserved.has(column));
|
||||
throw new UserError(
|
||||
selectable.length > 0
|
||||
? `Index ${name} has no column ${this.contentColumn}. Select a Content Column from: ${selectable.join(', ')}`
|
||||
: `Index ${name} has no column ${this.contentColumn}, and declares no other column to select`,
|
||||
);
|
||||
}
|
||||
return vectorColumn;
|
||||
}
|
||||
|
||||
// Databricks enforces `index_type` here; a Delta Sync index rejects upsert-data even when it holds its own vectors
|
||||
private assertDirectAccess(): string {
|
||||
const { indexType, embedding, name } = this.index;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
/* eslint-disable n8n-nodes-base/node-param-display-name-miscased */
|
||||
/* eslint-disable n8n-nodes-base/node-param-display-name-miscased-id */
|
||||
import type { Embeddings } from '@langchain/core/embeddings';
|
||||
import type { Tool } from '@langchain/core/tools';
|
||||
import { proxyFetch } from '@n8n/ai-utilities';
|
||||
import { DATABRICKS_PARTNER_USER_AGENT } from 'n8n-nodes-base/dist/nodes/Databricks/constants';
|
||||
import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers';
|
||||
|
|
@ -327,6 +328,60 @@ describe('VectorStoreDatabricks', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('supplyData in retrieve-as-tool mode', () => {
|
||||
// A managed-embedding index rejects query_vector, so the agent's query has to reach the
|
||||
// store as text. The factory picks that path from this node's `searchByText`.
|
||||
it('searches the agent query as text and never embeds it', async () => {
|
||||
const store = { similaritySearchWithScore: vi.fn().mockResolvedValue([]) };
|
||||
mockedFromExistingIndex.mockResolvedValue(store as unknown as DatabricksVectorStore);
|
||||
const ctx = setupContext<ISupplyDataFunctions>({
|
||||
...baseParams,
|
||||
mode: 'retrieve-as-tool',
|
||||
toolDescription: 'Company policies. Use for any policy question.',
|
||||
topK: 3,
|
||||
});
|
||||
ctx.addInputData = vi.fn().mockReturnValue({ index: 0 });
|
||||
ctx.addOutputData = vi.fn();
|
||||
|
||||
const { response } = await node.supplyData.call(ctx, 0);
|
||||
await (response as Tool).invoke({ input: 'what is the leave policy' });
|
||||
|
||||
expect(store.similaritySearchWithScore).toHaveBeenCalledWith(
|
||||
'what is the leave policy',
|
||||
3,
|
||||
undefined,
|
||||
);
|
||||
expect(embeddings.embedQuery).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// An agent-driven search must use the node's own configuration. The community package
|
||||
// drops the mode and the filter on this path, which is the regression this pins.
|
||||
it('carries the configured search mode, metadata filter and limit into the tool search', async () => {
|
||||
const store = { similaritySearchWithScore: vi.fn().mockResolvedValue([]) };
|
||||
mockedFromExistingIndex.mockResolvedValue(store as unknown as DatabricksVectorStore);
|
||||
const ctx = setupContext<ISupplyDataFunctions>({
|
||||
...baseParams,
|
||||
mode: 'retrieve-as-tool',
|
||||
toolDescription: 'Company policies. Use for any policy question.',
|
||||
topK: 3,
|
||||
options: { searchMode: 'HYBRID', searchFilterJson: { source: 'hr' } },
|
||||
});
|
||||
ctx.addInputData = vi.fn().mockReturnValue({ index: 0 });
|
||||
ctx.addOutputData = vi.fn();
|
||||
|
||||
const { response } = await node.supplyData.call(ctx, 0);
|
||||
await (response as Tool).invoke({ input: 'what is the leave policy' });
|
||||
|
||||
expect(mockedFromExistingIndex).toHaveBeenCalledWith(
|
||||
embeddings,
|
||||
expect.objectContaining({ queryType: 'HYBRID' }),
|
||||
);
|
||||
expect(store.similaritySearchWithScore).toHaveBeenCalledWith('what is the leave policy', 3, {
|
||||
source: 'hr',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('execute in insert mode', () => {
|
||||
it('adds the loaded documents through the store', async () => {
|
||||
const documents = [
|
||||
|
|
@ -369,7 +424,7 @@ describe('VectorStoreDatabricks', () => {
|
|||
let httpRequestWithAuthentication: ReturnType<typeof vi.fn>;
|
||||
let ctx: ILoadOptionsFunctions;
|
||||
|
||||
const setupSearchContext = (host: string) => {
|
||||
const setupSearchContext = (host: string, mode = 'load') => {
|
||||
httpRequestWithAuthentication = vi.fn(
|
||||
async (_type: string, options: { url: string; qs: Record<string, string> }) =>
|
||||
options.url.endsWith('/endpoints')
|
||||
|
|
@ -378,6 +433,7 @@ describe('VectorStoreDatabricks', () => {
|
|||
);
|
||||
ctx = {
|
||||
getCredentials: vi.fn().mockResolvedValue({ ...mockCredential, host }),
|
||||
getCurrentNodeParameter: vi.fn((path: string) => (path === 'mode' ? mode : undefined)),
|
||||
getNode: vi.fn().mockReturnValue(nodeDef),
|
||||
helpers: { httpRequestWithAuthentication },
|
||||
} as unknown as ILoadOptionsFunctions;
|
||||
|
|
@ -401,6 +457,21 @@ describe('VectorStoreDatabricks', () => {
|
|||
}
|
||||
});
|
||||
|
||||
// Only insert writes, so only insert may narrow the list; the read modes query both types.
|
||||
it.each([
|
||||
['insert', ['cat.sch.zeta']],
|
||||
['load', ['cat.sch.alpha', 'cat.sch.beta', 'cat.sch.zeta']],
|
||||
['retrieve', ['cat.sch.alpha', 'cat.sch.beta', 'cat.sch.zeta']],
|
||||
['retrieve-as-tool', ['cat.sch.alpha', 'cat.sch.beta', 'cat.sch.zeta']],
|
||||
])('offers the writable indexes in %s mode', async (mode, expected) => {
|
||||
setupSearchContext('https://ws.example.com/', mode);
|
||||
|
||||
const result = await methods.listSearch.searchIndexes.call(ctx);
|
||||
|
||||
expect(result.results.map((entry) => entry.value)).toEqual(expected);
|
||||
expect(ctx.getCurrentNodeParameter).toHaveBeenCalledWith('mode');
|
||||
});
|
||||
|
||||
it('does not let the bearer follow a cross-origin redirect', async () => {
|
||||
setupSearchContext('https://ws.example.com');
|
||||
|
||||
|
|
|
|||
|
|
@ -231,6 +231,7 @@ async function searchIndexes(
|
|||
this: ILoadOptionsFunctions,
|
||||
filter?: string,
|
||||
): Promise<INodeListSearchResult> {
|
||||
const insertOnly = this.getCurrentNodeParameter('mode') === 'insert';
|
||||
const credentials = await this.getCredentials<DatabricksOAuth2Credential>(
|
||||
DATABRICKS_CREDENTIAL_TYPE,
|
||||
);
|
||||
|
|
@ -252,12 +253,13 @@ async function searchIndexes(
|
|||
{ endpoint_name: endpoint.name },
|
||||
(page: ListPage) => page.vector_indexes,
|
||||
);
|
||||
// The type tells the user which indexes accept inserts
|
||||
return indexes.map((index) => ({
|
||||
name: index.name,
|
||||
value: index.name,
|
||||
description: `${index.index_type} - ${endpoint.name}`,
|
||||
}));
|
||||
return indexes
|
||||
.filter((index) => !insertOnly || index.index_type === 'DIRECT_ACCESS')
|
||||
.map((index) => ({
|
||||
name: index.name,
|
||||
value: index.name,
|
||||
description: `${index.index_type} - ${endpoint.name}`,
|
||||
}));
|
||||
}),
|
||||
)
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user