diff --git a/meerkat-browser/package.json b/meerkat-browser/package.json index ddf89312..e8d72931 100644 --- a/meerkat-browser/package.json +++ b/meerkat-browser/package.json @@ -1,6 +1,6 @@ { "name": "@devrev/meerkat-browser", - "version": "0.0.134", + "version": "0.0.135", "dependencies": { "tslib": "^2.3.0", "@devrev/meerkat-core": "*", diff --git a/meerkat-core/package.json b/meerkat-core/package.json index f74bf625..7d9c217d 100644 --- a/meerkat-core/package.json +++ b/meerkat-core/package.json @@ -1,6 +1,6 @@ { "name": "@devrev/meerkat-core", - "version": "0.0.134", + "version": "0.0.135", "dependencies": { "tslib": "^2.3.0" }, diff --git a/meerkat-core/src/joins/v2/joins.spec.ts b/meerkat-core/src/joins/v2/joins.spec.ts index 04be0d9d..d9fea489 100644 --- a/meerkat-core/src/joins/v2/joins.spec.ts +++ b/meerkat-core/src/joins/v2/joins.spec.ts @@ -1,4 +1,8 @@ -import { MeerkatQueryFilter, StructuredJoin, TableSchema } from '../../types/cube-types'; +import { + MeerkatQueryFilter, + StructuredJoin, + TableSchema, +} from '../../types/cube-types'; import { GetQueryOutput } from '../../utils/duckdb-ast-parse-serialize'; import { createDirectedGraphV2, generateSqlQueryV2 } from './joins'; @@ -45,7 +49,10 @@ const sqlMapOf = (schemas: TableSchema[]): { [k: string]: string } => describe('joins-v2', () => { it('emits a plain equi-join when from is scalar', async () => { - const schemas = [scalar('orders', ['id', 'customer_id']), scalar('customers')]; + const schemas = [ + scalar('orders', ['id', 'customer_id']), + scalar('customers'), + ]; const sqlMap = sqlMapOf(schemas); const paths: StructuredJoin[][] = [ [ @@ -247,14 +254,33 @@ describe('joins-v2', () => { const bridgeTables = new Set(['link']); const paths: StructuredJoin[][] = [ [ - { from: { table: 'issue', column: 'id' }, to: { table: 'link', column: 'source_id' } }, - { from: { table: 'link', column: 'target_id' }, to: { table: 'part', column: 'id' } }, - { from: { table: 'issue', column: 'id' }, to: { table: 'link', column: 'source_id' } }, - { from: { table: 'link', column: 'target_id' }, to: { table: 'user', column: 'id' } }, + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'source_id' }, + }, + { + from: { table: 'link', column: 'target_id' }, + to: { table: 'part', column: 'id' }, + }, + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'source_id' }, + }, + { + from: { table: 'link', column: 'target_id' }, + to: { table: 'user', column: 'id' }, + }, ], ]; const graph = createDirectedGraphV2(schemas, sqlMap, paths); - const sql = await generateSqlQueryV2(paths, sqlMap, graph, schemas, undefined, bridgeTables); + const sql = await generateSqlQueryV2( + paths, + sqlMap, + graph, + schemas, + undefined, + bridgeTables + ); expect(sql).toContain('issue.id = link.source_id'); expect(sql).toContain('link.target_id = part.id'); @@ -273,16 +299,35 @@ describe('joins-v2', () => { const bridgeTables = new Set(['link']); const paths: StructuredJoin[][] = [ [ - { from: { table: 'issue', column: 'id' }, to: { table: 'link', column: 'source_id' } }, - { from: { table: 'link', column: 'target_id' }, to: { table: 'part', column: 'id' } }, + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'source_id' }, + }, + { + from: { table: 'link', column: 'target_id' }, + to: { table: 'part', column: 'id' }, + }, ], [ - { from: { table: 'issue', column: 'id' }, to: { table: 'link', column: 'source_id' } }, - { from: { table: 'link', column: 'target_id' }, to: { table: 'user', column: 'id' } }, + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'source_id' }, + }, + { + from: { table: 'link', column: 'target_id' }, + to: { table: 'user', column: 'id' }, + }, ], ]; const graph = createDirectedGraphV2(schemas, sqlMap, paths); - const sql = await generateSqlQueryV2(paths, sqlMap, graph, schemas, undefined, bridgeTables); + const sql = await generateSqlQueryV2( + paths, + sqlMap, + graph, + schemas, + undefined, + bridgeTables + ); expect(sql).toContain('issue.id = link.source_id'); expect(sql).toContain('link.target_id = part.id'); @@ -299,9 +344,18 @@ describe('joins-v2', () => { const sqlMap = sqlMapOf(schemas); const paths: StructuredJoin[][] = [ [ - { from: { table: 'issue', column: 'id' }, to: { table: 'link', column: 'source_id' } }, - { from: { table: 'link', column: 'target_id' }, to: { table: 'part', column: 'id' } }, - { from: { table: 'issue', column: 'id' }, to: { table: 'link', column: 'source_id' } }, + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'source_id' }, + }, + { + from: { table: 'link', column: 'target_id' }, + to: { table: 'part', column: 'id' }, + }, + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'source_id' }, + }, ], ]; const graph = createDirectedGraphV2(schemas, sqlMap, paths); @@ -440,12 +494,101 @@ describe('joins-v2', () => { ); expect(callCount).toBe(1); - expect(sql).toContain("issue.id = link.source_id AND (link_type_id = 'type_a')"); + expect(sql).toContain( + "issue.id = link.source_id AND (link_type_id = 'type_a')" + ); expect(sql).toContain('link.target_id = part.id'); - expect(sql).toContain("part.id = link__1.source_id AND (link_type_id = 'type_b')"); + expect(sql).toContain( + "part.id = link__1.source_id AND (link_type_id = 'type_b')" + ); expect(sql).toContain('link__1.target_id = user.id'); }); + it('rewrites condition members to the aliased bridge table (link__1)', async () => { + // Reproduces: Issue → link (Child of) → Enhancement and Issue → link + // (Dependency of) → Ticket. The second link join must filter on + // link__1.link_type_id, not the first link's link_type_id. + const schemas = [ + scalar('issue', ['id']), + scalar('link', ['id', 'source_id', 'target_id', 'link_type_id']), + scalar('enhancement', ['id', 'name']), + scalar('ticket', ['id', 'severity']), + ]; + const sqlMap = sqlMapOf(schemas); + const bridgeTables = new Set(['link']); + const paths: StructuredJoin[][] = [ + [ + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'target_id' }, + condition: { + member: 'link.link_type_id', + operator: 'equals', + values: ['custom_link_type/20'], + }, + }, + { + from: { table: 'link', column: 'source_id' }, + to: { table: 'enhancement', column: 'id' }, + }, + ], + [ + { + from: { table: 'issue', column: 'id' }, + to: { table: 'link', column: 'target_id' }, + condition: { + member: 'link.link_type_id', + operator: 'equals', + values: ['custom_link_type/default-20'], + }, + }, + { + from: { table: 'link', column: 'source_id' }, + to: { table: 'ticket', column: 'id' }, + }, + ], + ]; + const graph = createDirectedGraphV2(schemas, sqlMap, paths); + + let serializedAstQuery = ''; + const mockGetQueryOutput: GetQueryOutput = async (query) => { + serializedAstQuery = query; + return [ + { + result: + "SELECT (link.link_type_id = 'custom_link_type/20') AS __meerkat_batch_expr_0__, (link__1.link_type_id = 'custom_link_type/default-20') AS __meerkat_batch_expr_1__;", + }, + ]; + }; + + const sql = await generateSqlQueryV2( + paths, + sqlMap, + graph, + schemas, + mockGetQueryOutput, + bridgeTables + ); + + // Condition AST must reference the aliased instance — this is what + // production DuckDB serialization emits as link__1.link_type_id. + expect(serializedAstQuery).toContain( + '"column_names":["link","link_type_id"]' + ); + expect(serializedAstQuery).toContain( + '"column_names":["link__1","link_type_id"]' + ); + expect(sql).toContain( + "issue.id = link.target_id AND (link.link_type_id = 'custom_link_type/20')" + ); + expect(sql).toContain( + "issue.id = link__1.target_id AND (link__1.link_type_id = 'custom_link_type/default-20')" + ); + expect(sql).not.toContain( + "issue.id = link__1.target_id AND (link.link_type_id = 'custom_link_type/default-20')" + ); + }); + it('handles notSet condition (IS NULL)', async () => { const schemas = [ scalar('issue', ['id']), @@ -526,7 +669,8 @@ describe('joins-v2', () => { ); expect(sql).toContain('issue.id = link.source_id AND'); - expect(sql).toContain("(link_type_id = 'type_a') OR (link_type_id = 'type_b')"); + expect(sql).toContain( + "(link_type_id = 'type_a') OR (link_type_id = 'type_b')" + ); }); - }); diff --git a/meerkat-core/src/joins/v2/joins.ts b/meerkat-core/src/joins/v2/joins.ts index 1699d2b9..5e075b38 100644 --- a/meerkat-core/src/joins/v2/joins.ts +++ b/meerkat-core/src/joins/v2/joins.ts @@ -2,6 +2,7 @@ import { cubeFilterToDuckdbAST } from '../../cube-filter-transformer/factory'; import { traverseMeerkatQueryFilter } from '../../filter-params/filter-params-ast'; import { getUsedTableSchema } from '../../get-used-table-schema/get-used-table-schema'; import { memberKeyToSafeKey } from '../../member-formatters/member-key-to-safe-key'; +import { splitIntoDataSourceAndFields } from '../../member-formatters/split-into-data-source-and-fields'; import { MeerkatQueryFilter, Query, @@ -199,6 +200,21 @@ const inferBridgeTables = ( return bridges; }; +const rewriteConditionTableAlias = ( + condition: MeerkatQueryFilter, + originalTable: string, + aliasedTable: string +): MeerkatQueryFilter => { + const rewritten: MeerkatQueryFilter = JSON.parse(JSON.stringify(condition)); + traverseMeerkatQueryFilter([rewritten], (filter) => { + const [table, fields] = splitIntoDataSourceAndFields(filter.member); + if (table === originalTable) { + filter.member = `${aliasedTable}.${fields}`; + } + }); + return rewritten; +}; + const aliasBridgeTables = ( paths: StructuredJoin[][], bridgeTables: Set @@ -225,10 +241,23 @@ const aliasBridgeTables = ( continue; } - const alias = `${edge.to.table}__${count}`; - result.push({ ...edge, to: { ...edge.to, table: alias } }); - - if (i + 1 < path.length && path[i + 1].from.table === edge.to.table) { + const originalTo = edge.to.table; + const alias = `${originalTo}__${count}`; + result.push({ + ...edge, + to: { ...edge.to, table: alias }, + ...(edge.condition + ? { + condition: rewriteConditionTableAlias( + edge.condition, + originalTo, + alias + ), + } + : {}), + }); + + if (i + 1 < path.length && path[i + 1].from.table === originalTo) { nextFromAlias = alias; } } diff --git a/meerkat-node/package.json b/meerkat-node/package.json index 9cea0ac3..d1b1d75b 100644 --- a/meerkat-node/package.json +++ b/meerkat-node/package.json @@ -1,6 +1,6 @@ { "name": "@devrev/meerkat-node", - "version": "0.0.134", + "version": "0.0.135", "dependencies": { "@devrev/meerkat-core": "*", "@swc/helpers": "~0.5.0",