From 26fdfb65957a5eee981e8b37b25a51ddc237e319 Mon Sep 17 00:00:00 2001 From: jayachandranramadoss-devrev Date: Thu, 4 Jun 2026 23:19:45 +0530 Subject: [PATCH 1/2] fix(meerkat-core): prune redundant post-join SELECT * to stop ambiguous CSV export (ISS-301213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSV export from dashboard widgets failed with `Binder Error: Ambiguous reference to column name ""` when the resolution-generated SQL left-joined a base query against multiple lookup tables that share a raw column name (e.g. `devrev_copy` across `dim_group` and `dim_devu`). The export wrapper emitted `SELECT , * FROM ()`; the trailing bare `*` re-exposed every joined table's raw columns, so a shared name became ambiguous and DuckDB rejected the COPY. The `*` is load-bearing — resolved lookup columns (e.g. `..__owner_id__fullname`) reach the outer query only through it, so it cannot simply be deleted. Add an AST pass, `pruneRedundantStarsOverJoins`, that: - serializes the final SQL via DuckDB `json_serialize_sql`, - finds each bare `*` whose enclosing SELECT reads from a JOIN, - replaces the `*` with explicit refs to the JOIN-exposed columns that are still referenced elsewhere and not already projected, dropping the unreferenced raw passthrough (e.g. `devrev_copy`) that caused the collision. The rewrite is surgical: it edits only the `*` character (located via the node's `query_location` byte offset, converted to a UTF-16 index so multibyte literals don't shift it) and leaves the rest of the SQL byte-for-byte intact, so existing generated SQL is otherwise unchanged. Any serialize/parse failure returns the original SQL untouched, so the export path never throws. Wired into `cubeQueryToSQLWithResolution` at the resolution exit in both meerkat-node and meerkat-browser. The live-query path is unaffected (resolution is skipped there). Tests: unit coverage for the pass (byte-offset location, leading-star comma handling, graceful degradation on reject/parse-failure, EXCLUDE untouched) and a node integration regression asserting no bare `*` over a JOIN plus a working CSV COPY with resolved values preserved. The prod Binder error is not directly reproducible locally because the bundled DuckDB auto-renames duplicate `*` columns, so the regression oracle is SQL shape + successful execution. work-item: ISS-301213 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../browser-cube-to-sql-with-resolution.ts | 19 +- meerkat-core/src/index.ts | 1 + .../prune-redundant-stars.spec.ts | 228 ++++++++++++ .../prune-redundant-stars.ts | 334 ++++++++++++++++++ ...resolution-shared-column-ambiguity.spec.ts | 235 ++++++++++++ meerkat-node/src/__tests__/resolution.spec.ts | 27 +- .../cube-to-sql-with-resolution.ts | 12 +- 7 files changed, 831 insertions(+), 25 deletions(-) create mode 100644 meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts create mode 100644 meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts create mode 100644 meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts diff --git a/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts b/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts index 431d0dcb..72aebbeb 100644 --- a/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts +++ b/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts @@ -10,6 +10,7 @@ import { Dimension, generateRowNumberSql, memberKeyToSafeKey, + pruneRedundantStarsOverJoins, Query, ResolutionConfig, ROW_ID_DIMENSION_NAME, @@ -20,6 +21,14 @@ import { import { AsyncDuckDBConnection } from '@duckdb/duckdb-wasm'; import { cubeQueryToSQL } from '../browser-cube-to-sql/browser-cube-to-sql'; +const getQueryOutput = async ( + query: string, + connection: AsyncDuckDBConnection +): Promise[]> => { + const queryOutput = await connection.query(query); + return queryOutput.toArray().map((row) => row.toJSON() as Record); +}; + export interface CubeQueryToSQLWithResolutionParams { connection: AsyncDuckDBConnection; query: Query; @@ -165,8 +174,16 @@ export const cubeQueryToSQLWithResolution = async ({ }); // Wrap with row_id ordering and exclusion - return wrapWithRowIdOrderingAndExclusion( + const finalSql = wrapWithRowIdOrderingAndExclusion( sqlWithAliases, ROW_ID_DIMENSION_NAME ); + + // Drop redundant post-join `*` projections so shared lookup columns can't + // collide (ISS-301213). Failures here degrade gracefully to the original SQL. + const { sql: prunedSql } = await pruneRedundantStarsOverJoins( + finalSql, + (query) => getQueryOutput(query, connection) + ); + return prunedSql; }; diff --git a/meerkat-core/src/index.ts b/meerkat-core/src/index.ts index c91e1f4e..ed72c2d0 100644 --- a/meerkat-core/src/index.ts +++ b/meerkat-core/src/index.ts @@ -38,6 +38,7 @@ export { BASE_TABLE_NAME } from './utils/base-ast'; export * from './utils/duckdb-ast-parse-serialize'; export * from './utils/cube-to-table-schema'; export * from './utils/get-column-names-from-ast'; +export * from './prune-redundant-stars/prune-redundant-stars'; export * from './utils/get-possible-nodes'; export { meerkatPlaceholderReplacer } from './utils/meerkat-placeholder-replacer'; export { ensureColumnAliasBatch } from './utils/ensure-sql-expression-column-alias'; diff --git a/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts b/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts new file mode 100644 index 00000000..566d7f12 --- /dev/null +++ b/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts @@ -0,0 +1,228 @@ +import { pruneRedundantStarsOverJoins } from './prune-redundant-stars'; + +/** + * Builds a `json_serialize_sql` mock response for a single statement whose AST + * is `selectNode`. The pass only ever issues one serialize query, so the mock + * returns this for any input. + */ +const serializeResponse = (selectNode: unknown) => [ + { + json_serialize_sql: JSON.stringify({ + error: false, + statements: [{ node: selectNode }], + }), + }, +]; + +const columnRef = (name: string, location?: number) => ({ + class: 'COLUMN_REF', + type: 'COLUMN_REF', + alias: '', + column_names: [name], + ...(location !== undefined ? { query_location: location } : {}), +}); + +const bareStar = (location: number) => ({ + class: 'STAR', + type: 'STAR', + alias: '', + query_location: location, + relation_name: '', + exclude_list: [], + replace_list: [], + columns: false, + expr: null, +}); + +const subqueryTable = (alias: string, selectList: unknown[]) => ({ + type: 'SUBQUERY', + alias, + subquery: { node: { type: 'SELECT_NODE', select_list: selectList } }, +}); + +describe('pruneRedundantStarsOverJoins', () => { + it('replaces a bare star over a JOIN with the referenced, unprojected columns', async () => { + // SQL shape: + // SELECT base.id AS id, * FROM (base) LEFT JOIN (lookup) ... + // where an outer scope references `resolved_name` (carried only by `*`), + // while `devrev_copy` is referenced by nobody. + const starLocation = 22; + const sql = 'SELECT base.id AS id, * FROM base LEFT JOIN lookup'; + + const selectNode = { + type: 'SELECT_NODE', + select_list: [columnRef('id', 7), bareStar(starLocation)], + from_table: { + type: 'JOIN', + left: subqueryTable('base', [columnRef('id')]), + // The lookup projects `resolved_name` explicitly and passes its raw + // columns (incl. `devrev_copy`) through an inner star — exactly the real + // shape. `devrev_copy` is therefore never a COLUMN_REF anywhere. + right: subqueryTable('lookup', [columnRef('resolved_name'), bareStar(99)]), + }, + // An outer reference to `resolved_name` so it counts as "needed". + where_clause: columnRef('resolved_name'), + }; + + const executeQuery = jest.fn(async () => serializeResponse(selectNode)); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(1); + // `resolved_name` is referenced and not already projected → kept. + // `id` is already projected → skipped. `devrev_copy` unreferenced → dropped. + expect(result.sql).toBe( + 'SELECT base.id AS id, "resolved_name" FROM base LEFT JOIN lookup' + ); + }); + + it('drops the star (and its comma) when no extra columns are needed', async () => { + const starLocation = 22; + const sql = 'SELECT base.id AS id, * FROM base LEFT JOIN lookup'; + + const selectNode = { + type: 'SELECT_NODE', + select_list: [columnRef('id', 7), bareStar(starLocation)], + from_table: { + type: 'JOIN', + left: subqueryTable('base', [columnRef('id')]), + // Lookup exposes only raw passthrough columns (via an inner star) that + // nothing references → the outer star contributes nothing. + right: subqueryTable('lookup', [bareStar(99)]), + }, + // Nothing outside references the lookup columns. + }; + + const executeQuery = jest.fn(async () => serializeResponse(selectNode)); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(1); + // `id` already projected, `devrev_copy` unreferenced → star removed cleanly. + expect(result.sql).toBe('SELECT base.id AS id FROM base LEFT JOIN lookup'); + }); + + it('locates the star by BYTE offset when a multibyte char precedes it', async () => { + // `query_location` is a UTF-8 byte offset. The 'é' in the literal makes the + // star's byte offset (21) differ from its UTF-16 index (20). Indexing the + // JS string with the raw byte offset would miss the star and silently skip + // the fix — the exact ISS-301213 failure on customer data with accents. + const sql = "SELECT 'café' AS c, * FROM base LEFT JOIN lookup"; + expect(sql.indexOf('*')).toBe(20); + const starByteOffset = 21; + + const selectNode = { + type: 'SELECT_NODE', + select_list: [columnRef('c', 7), bareStar(starByteOffset)], + from_table: { + type: 'JOIN', + left: subqueryTable('base', [columnRef('c')]), + right: subqueryTable('lookup', [columnRef('resolved_name'), bareStar(99)]), + }, + where_clause: columnRef('resolved_name'), + }; + + const executeQuery = jest.fn(async () => serializeResponse(selectNode)); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(1); + expect(result.sql).toBe( + "SELECT 'café' AS c, \"resolved_name\" FROM base LEFT JOIN lookup" + ); + }); + + it('strips a following comma when a dropped star is first in the list', async () => { + // Leading star with an empty replacement must not leave `SELECT , id`. + const sql = 'SELECT *, base.id AS id FROM base LEFT JOIN lookup'; + const starByteOffset = 7; + + const selectNode = { + type: 'SELECT_NODE', + select_list: [bareStar(starByteOffset), columnRef('id', 10)], + from_table: { + type: 'JOIN', + left: subqueryTable('base', [columnRef('id')]), + right: subqueryTable('lookup', [bareStar(99)]), + }, + }; + + const executeQuery = jest.fn(async () => serializeResponse(selectNode)); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(1); + expect(result.sql).toBe('SELECT base.id AS id FROM base LEFT JOIN lookup'); + }); + + it('returns the original SQL when executeQuery rejects', async () => { + const sql = 'SELECT base.id AS id, * FROM base LEFT JOIN lookup'; + const executeQuery = jest.fn(async () => { + throw new Error('DuckDB connection lost'); + }); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(0); + expect(result.sql).toBe(sql); + }); + + it('leaves a lone `SELECT *` over a single table untouched', async () => { + const sql = 'SELECT * FROM base'; + const selectNode = { + type: 'SELECT_NODE', + select_list: [bareStar(7)], + from_table: { type: 'BASE_TABLE', table_name: 'base' }, + }; + const executeQuery = jest.fn(async () => serializeResponse(selectNode)); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(0); + expect(result.sql).toBe(sql); + }); + + it('does not touch a star with an EXCLUDE list', async () => { + const sql = 'SELECT * EXCLUDE (x), a FROM base LEFT JOIN lookup'; + const selectNode = { + type: 'SELECT_NODE', + select_list: [ + { + class: 'STAR', + type: 'STAR', + alias: '', + query_location: 7, + relation_name: '', + exclude_list: ['x'], + replace_list: [], + columns: false, + expr: null, + }, + columnRef('a', 25), + ], + from_table: { + type: 'JOIN', + left: subqueryTable('base', [columnRef('a')]), + right: subqueryTable('lookup', [columnRef('b')]), + }, + }; + const executeQuery = jest.fn(async () => serializeResponse(selectNode)); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(0); + expect(result.sql).toBe(sql); + }); + + it('returns the original SQL when DuckDB cannot parse it', async () => { + const sql = 'not valid sql'; + const executeQuery = jest.fn(async () => [ + { json_serialize_sql: JSON.stringify({ error: true }) }, + ]); + + const result = await pruneRedundantStarsOverJoins(sql, executeQuery); + + expect(result.rewrittenStars).toBe(0); + expect(result.sql).toBe(sql); + }); +}); diff --git a/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts b/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts new file mode 100644 index 00000000..4b77ecf8 --- /dev/null +++ b/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts @@ -0,0 +1,334 @@ +import { astSerializerQuery } from '../ast-serializer/ast-serializer'; +import { deserializeQuery } from '../ast-deserializer/ast-deserializer'; +import { sanitizeStringValue } from '../member-formatters/sanitize-value'; +import { GetQueryOutput } from '../utils/duckdb-ast-parse-serialize'; + +/** + * Replaces redundant `SELECT , * FROM ()` star projections with an + * explicit, collision-free column list (ISS-301213). + * + * ## The bug + * + * The resolution pipeline wraps each layer as + * `SELECT , * FROM ()`. When `` + * is a multi-table JOIN (the base query left-joined against several lookup + * tables) the trailing `*` re-exposes the raw columns of *every* joined table. + * If two lookups share a column name (e.g. `devrev_copy` across `dim_group` + + * `dim_devu`), that name becomes ambiguous and DuckDB rejects the export with + * `Binder Error: Ambiguous reference to column name ...`. + * + * ## Why we cannot simply delete the star + * + * The star is load-bearing: the resolved lookup columns (e.g. + * `..__owner_id__fullname`) are produced inside the joined subqueries and reach + * the outer query *only* through it. Deleting it drops those columns + * ("Referenced column ... not found"). The star must be REPLACED by the exact + * columns the rest of the statement still references. + * + * ## Surgical, formatting-preserving rewrite + * + * We parse the SQL into DuckDB's AST only to *analyse* it, then edit the + * ORIGINAL SQL string in place using the `query_location` byte offset DuckDB + * attaches to every star node. Each offending `*` character is replaced with an + * explicit column list; every other byte of the SQL is left untouched. This + * avoids re-serializing the whole statement (which would cosmetically rewrite + * keyword case, quoting, parenthesisation, etc.). + * + * ## Algorithm + * + * 1. Collect every column referenced anywhere in the statement (`globalRefs`). + * 2. Find each bare star whose enclosing `SELECT` reads from a JOIN. + * 3. Compute the column names that JOIN exposes (from its child subqueries). + * 4. Replace the star with the exposed columns that are referenced somewhere + * and not already projected by a sibling. Columns nobody references — the + * ambiguous raw passthrough like `devrev_copy` — are left out, removing the + * collision while preserving every needed column. + * + * Bare star = a `STAR` with no `EXCLUDE` / `REPLACE` / `relation_name`. Those + * are intentional and are never touched, and a lone `SELECT *` is left as-is. + */ + +const STAR_CLASS = 'STAR'; +const COLUMN_REF_CLASS = 'COLUMN_REF'; +const SELECT_NODE_TYPE = 'SELECT_NODE'; +const JOIN_TABLE_TYPE = 'JOIN'; +const SUBQUERY_TABLE_TYPE = 'SUBQUERY'; + +interface AstNode { + class?: string; + type?: string; + alias?: string; + relation_name?: string; + exclude_list?: unknown[]; + replace_list?: unknown[]; + columns?: boolean; + expr?: unknown; + column_names?: string[]; + query_location?: number; + select_list?: AstNode[]; + from_table?: AstNode; + left?: AstNode; + right?: AstNode; + subquery?: { node?: AstNode }; + [key: string]: unknown; +} + +const isBareStar = (item: AstNode): boolean => { + return ( + item?.class === STAR_CLASS && + !item.relation_name && + (!item.exclude_list || item.exclude_list.length === 0) && + (!item.replace_list || item.replace_list.length === 0) && + !item.columns && + !item.expr + ); +}; + +/** Last segment of a `COLUMN_REF`'s `column_names` (the bare column name). */ +const refName = (item: AstNode): string | undefined => { + if (item?.class !== COLUMN_REF_CLASS || !Array.isArray(item.column_names)) { + return undefined; + } + return item.column_names[item.column_names.length - 1]; +}; + +/** Every bare column name referenced by a `COLUMN_REF` anywhere under `root`. */ +const collectGlobalColumnRefs = (root: unknown): Set => { + const refs = new Set(); + const stack: unknown[] = [root]; + while (stack.length > 0) { + const current = stack.pop(); + if (!current || typeof current !== 'object') continue; + if (Array.isArray(current)) { + current.forEach((item) => stack.push(item)); + continue; + } + const record = current as AstNode; + const name = refName(record); + if (name) refs.add(name); + Object.values(record).forEach((value) => stack.push(value)); + } + return refs; +}; + +/** + * The set of column names a FROM table exposes upward that we can ENUMERATE. + * + * - A JOIN exposes the union of both sides' enumerable columns. + * - A SUBQUERY exposes the explicit aliases/column-names in its select list. If + * that select list itself contains a star, the star contributes an unknown + * set of raw columns — never referenced by the resolution pipeline — so we + * skip it rather than bailing out. + * - Base tables (and anything else) expose a non-enumerable set → `null`, which + * tells the caller to leave the star untouched. + */ +const getExposedColumns = (table: AstNode | null | undefined): string[] | null => { + if (!table) return null; + + if (table.type === JOIN_TABLE_TYPE) { + const left = getExposedColumns(table.left); + const right = getExposedColumns(table.right); + if (left === null || right === null) return null; + return [...left, ...right]; + } + + if (table.type === SUBQUERY_TABLE_TYPE) { + const node = table.subquery?.node; + if (!node || !Array.isArray(node.select_list)) return null; + const names: string[] = []; + for (const item of node.select_list) { + if (item?.class === STAR_CLASS) { + // Inner star exposes raw passthrough columns we cannot enumerate; they + // are never referenced upward in the resolution pipeline, so omitting + // them is safe — skip rather than abort. + continue; + } + const columnName = item?.alias || refName(item); + if (!columnName) return null; + names.push(columnName); + } + return names; + } + + return null; +}; + +interface StarEdit { + location: number; + replacement: string; +} + +const quoteIdentifier = (name: string): string => `"${name.replace(/"/g, '""')}"`; + +/** + * Walks the AST collecting a string edit for each bare-star-over-JOIN node: the + * byte offset of its `*` and the explicit column list that should replace it. + * Stars whose needed-column set is empty (everything already projected) or + * whose JOIN columns cannot be enumerated are skipped. + */ +const collectStarEdits = (root: unknown, globalRefs: Set): StarEdit[] => { + const edits: StarEdit[] = []; + const stack: unknown[] = [root]; + + while (stack.length > 0) { + const current = stack.pop(); + if (!current || typeof current !== 'object') continue; + if (Array.isArray(current)) { + current.forEach((item) => stack.push(item)); + continue; + } + const record = current as AstNode; + + if ( + record.type === SELECT_NODE_TYPE && + Array.isArray(record.select_list) && + record.from_table?.type === JOIN_TABLE_TYPE + ) { + const star = record.select_list.find( + (item: AstNode) => isBareStar(item) && typeof item.query_location === 'number' + ); + + if (star) { + const exposed = getExposedColumns(record.from_table); + if (exposed !== null) { + const alreadyProjected = new Set( + record.select_list + .map(refName) + .filter((n: string | undefined): n is string => Boolean(n)) + ); + + const seen = new Set(); + const needed: string[] = []; + for (const name of exposed) { + if ( + globalRefs.has(name) && + !alreadyProjected.has(name) && + !seen.has(name) + ) { + seen.add(name); + needed.push(name); + } + } + + // Replace `*` with the explicit needed columns. If none are needed, + // drop the `*` entirely by collapsing the preceding ", " — handled by + // the caller via an empty replacement marker. + if (typeof star.query_location === 'number') { + edits.push({ + location: star.query_location, + replacement: needed.map(quoteIdentifier).join(', '), + }); + } + } + } + } + + Object.values(record).forEach((value) => stack.push(value)); + } + + return edits; +}; + +/** + * Converts a DuckDB `query_location` (a UTF-8 BYTE offset) into a JS string + * (UTF-16 code-unit) index. They diverge whenever the SQL contains a multi-byte + * character (e.g. an accented name or emoji in a literal) before the offset, so + * indexing the JS string directly with the raw byte offset would point at the + * wrong character. + */ +const byteOffsetToCharIndex = (sql: string, byteOffset: number): number => { + return Buffer.from(sql, 'utf8').subarray(0, byteOffset).toString('utf8').length; +}; + +/** + * Applies star edits to the original SQL string. Each edit replaces the single + * `*` character (located via its byte offset, converted to a char index) with + * the explicit column list. When the replacement is empty (no columns needed), + * the comma separating the star from its neighbour is removed too — the + * preceding `, ` if present, otherwise a following `, ` (covers a leading star + * such as `SELECT *, x`) — so we never leave a dangling comma. Edits are + * applied right-to-left so earlier offsets stay valid. + */ +const applyStarEdits = (sql: string, edits: StarEdit[]): string => { + const ordered = [...edits].sort((a, b) => b.location - a.location); + let result = sql; + + for (const edit of ordered) { + const at = byteOffsetToCharIndex(result, edit.location); + if (result[at] !== '*') { + // Offset no longer points at a star (defensive); skip this edit. + continue; + } + + if (edit.replacement.length > 0) { + result = result.slice(0, at) + edit.replacement + result.slice(at + 1); + continue; + } + + // Empty replacement: drop the `*` and the comma that separated it from a + // sibling projection. Prefer the preceding `, `; if the star was first in + // the list (`SELECT *, x`), strip the following `, ` instead. + const before = result.slice(0, at); + const after = result.slice(at + 1); + const precedingComma = before.match(/,\s*$/); + if (precedingComma) { + result = result.slice(0, at - precedingComma[0].length) + after; + continue; + } + const followingComma = after.match(/^\s*,\s*/); + if (followingComma) { + result = before + after.slice(followingComma[0].length); + continue; + } + // No separating comma (lone star) — just remove the `*`. + result = before + after; + } + + return result; +}; + +export interface PruneRedundantStarsResult { + sql: string; + rewrittenStars: number; +} + +/** + * Parses `sql` into a DuckDB AST, then surgically rewrites each redundant + * post-join star projection into an explicit collision-free column list, + * editing the original SQL string in place (formatting preserved). On any + * failure — DuckDB rejecting the serialize query, a malformed serialization, + * or a parse error — the original SQL is returned unchanged so the export path + * degrades gracefully (it never throws). + * + * @param sql - the fully-assembled resolution/export SQL + * @param executeQuery - runs a query against DuckDB and returns its rows + */ +export const pruneRedundantStarsOverJoins = async ( + sql: string, + executeQuery: GetQueryOutput +): Promise => { + let serialized: { error?: boolean; statements?: AstNode[] }; + try { + const serializedRows = await executeQuery( + astSerializerQuery(sanitizeStringValue(sql)) + ); + serialized = JSON.parse(deserializeQuery(serializedRows)); + } catch { + // DuckDB rejected the serialize query or the payload could not be parsed — + // leave the SQL untouched rather than breaking the export. + return { sql, rewrittenStars: 0 }; + } + + if (serialized.error || !Array.isArray(serialized.statements)) { + return { sql, rewrittenStars: 0 }; + } + + const globalRefs = collectGlobalColumnRefs(serialized.statements); + const edits = collectStarEdits(serialized.statements, globalRefs); + if (edits.length === 0) { + return { sql, rewrittenStars: 0 }; + } + + const rewrittenSql = applyStarEdits(sql, edits); + return { sql: rewrittenSql, rewrittenStars: edits.length }; +}; diff --git a/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts b/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts new file mode 100644 index 00000000..6aa0e8e4 --- /dev/null +++ b/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts @@ -0,0 +1,235 @@ +import { Query, ResolutionConfig, TableSchema } from '@devrev/meerkat-core'; +import { cubeQueryToSQLWithResolution } from '../cube-to-sql-with-resolution/cube-to-sql-with-resolution'; +import { duckdbExec } from '../duckdb-exec'; + +/** + * Counts bare `*` projections that sit over a multi-table (JOIN) scope in the + * generated SQL by parsing it through DuckDB's AST serializer. + * + * A "bare" STAR is `SELECT ..., * FROM ...` with no EXCLUDE/REPLACE — the exact + * shape that re-exposes raw columns from every joined table and triggers the + * ISS-301213 ambiguity. Counting via the AST (not regex) keeps the assertion + * robust against formatting and string literals. + * + * The environment's DuckDB auto-renames duplicate `*` columns, so the Binder + * error itself is not reproducible here. The deterministic, version-independent + * oracle is therefore the SQL SHAPE: no bare STAR over a JOIN scope. + */ +const countBareStarsOverJoins = async (sql: string): Promise => { + const rows = (await duckdbExec( + `SELECT json_serialize_sql('${sql.replace(/'/g, "''")}')` + )) as Record[]; + const ast = JSON.parse(Object.values(rows[0])[0]); + + let count = 0; + const visit = (node: unknown): void => { + if (!node || typeof node !== 'object') return; + const record = node as Record; + + if (record.type === 'SELECT_NODE' && Array.isArray(record.select_list)) { + const hasBareStar = record.select_list.some( + (item: any) => + item?.class === 'STAR' && + (!item.exclude_list || item.exclude_list.length === 0) && + !item.relation_name + ); + const fromIsJoin = record.from_table?.type === 'JOIN'; + if (hasBareStar && fromIsJoin) { + count += 1; + } + } + + if (Array.isArray(node)) { + node.forEach(visit); + return; + } + Object.values(record).forEach(visit); + }; + visit(ast); + return count; +}; + +/** + * Regression test for ISS-301213. + * + * CSV export from a dashboard widget fails with + * `Binder Error: Ambiguous reference to column name ""` + * when the widget's SQL resolves two (or more) scalar lookup columns whose + * source lookup tables share a column name beyond the one being resolved. + * + * Real-world shape (from the Razorpay HAR on TKT-65015): a `dim_ticket` base + * query left-joins lookup tables `dim_devu` (owner) and `dim_group`, both of + * which carry an unrelated column `devrev_copy`. The export wrapper produced a + * trailing bare `*` that re-expanded BOTH lookup tables' columns into the same + * scope, so `devrev_copy` became ambiguous and DuckDB's binder rejected the + * COPY statement. + * + * The shared column is NOT projected and NOT resolved — it is pure passthrough + * dragged up by the `*`. The fix removes that redundant passthrough `*` on the + * resolution/export path (the named projections already enumerate every output + * column), so a shared lookup column can no longer collide. + */ + +const CREATE_BASE_TABLE = `CREATE OR REPLACE TABLE iss301213_tickets ( + id INTEGER, + owner_id VARCHAR, + group_id VARCHAR +)`; + +const BASE_DATA = `INSERT INTO iss301213_tickets VALUES +(1, 'owner1', 'group1'), +(2, 'owner2', 'group2'), +(3, 'owner1', 'group2')`; + +// Owner lookup carries an extra `devrev_copy` column that is never resolved. +const CREATE_OWNER_LOOKUP = `CREATE OR REPLACE TABLE iss301213_owner_lookup ( + id VARCHAR, + fullname VARCHAR, + devrev_copy VARCHAR +)`; + +const OWNER_LOOKUP_DATA = `INSERT INTO iss301213_owner_lookup VALUES +('owner1', 'Alice', 'copy-o1'), +('owner2', 'Bob', 'copy-o2')`; + +// Group lookup ALSO carries `devrev_copy` — this is the shared column that +// becomes ambiguous when both lookups are joined and re-exposed via `*`. +const CREATE_GROUP_LOOKUP = `CREATE OR REPLACE TABLE iss301213_group_lookup ( + id VARCHAR, + name VARCHAR, + devrev_copy VARCHAR +)`; + +const GROUP_LOOKUP_DATA = `INSERT INTO iss301213_group_lookup VALUES +('group1', 'Group One', 'copy-g1'), +('group2', 'Group Two', 'copy-g2')`; + +const BASE_SCHEMA: TableSchema = { + name: 'iss301213_tickets', + sql: 'select * from iss301213_tickets', + measures: [{ name: 'count', sql: 'COUNT(*)', type: 'number' }], + dimensions: [ + { alias: 'ID', name: 'id', sql: 'id', type: 'number' }, + { alias: 'Owner', name: 'owner_id', sql: 'owner_id', type: 'string' }, + { alias: 'Group', name: 'group_id', sql: 'group_id', type: 'string' }, + ], +}; + +const OWNER_LOOKUP_SCHEMA: TableSchema = { + name: 'iss301213_owner_lookup', + sql: 'select * from iss301213_owner_lookup', + measures: [], + dimensions: [ + { alias: 'ID', name: 'id', sql: 'id', type: 'string' }, + { alias: 'Full Name', name: 'fullname', sql: 'fullname', type: 'string' }, + { + alias: 'Devrev Copy', + name: 'devrev_copy', + sql: 'devrev_copy', + type: 'string', + }, + ], +}; + +const GROUP_LOOKUP_SCHEMA: TableSchema = { + name: 'iss301213_group_lookup', + sql: 'select * from iss301213_group_lookup', + measures: [], + dimensions: [ + { alias: 'ID', name: 'id', sql: 'id', type: 'string' }, + { alias: 'Name', name: 'name', sql: 'name', type: 'string' }, + { + alias: 'Devrev Copy', + name: 'devrev_copy', + sql: 'devrev_copy', + type: 'string', + }, + ], +}; + +describe('ISS-301213 - shared lookup column ambiguity on CSV export', () => { + beforeAll(async () => { + await duckdbExec(CREATE_BASE_TABLE); + await duckdbExec(BASE_DATA); + await duckdbExec(CREATE_OWNER_LOOKUP); + await duckdbExec(OWNER_LOOKUP_DATA); + await duckdbExec(CREATE_GROUP_LOOKUP); + await duckdbExec(GROUP_LOOKUP_DATA); + }); + + afterAll(async () => { + await duckdbExec('DROP TABLE IF EXISTS iss301213_tickets'); + await duckdbExec('DROP TABLE IF EXISTS iss301213_owner_lookup'); + await duckdbExec('DROP TABLE IF EXISTS iss301213_group_lookup'); + }); + + const query: Query = { + measures: ['iss301213_tickets.count'], + dimensions: [ + 'iss301213_tickets.id', + 'iss301213_tickets.owner_id', + 'iss301213_tickets.group_id', + ], + order: { 'iss301213_tickets.id': 'asc' }, + }; + + const resolutionConfig: ResolutionConfig = { + columnConfigs: [ + { + name: 'iss301213_tickets.owner_id', + type: 'string' as const, + source: 'iss301213_owner_lookup', + joinColumn: 'id', + resolutionColumns: ['fullname'], + }, + { + name: 'iss301213_tickets.group_id', + type: 'string' as const, + source: 'iss301213_group_lookup', + joinColumn: 'id', + resolutionColumns: ['name'], + }, + ], + tableSchemas: [OWNER_LOOKUP_SCHEMA, GROUP_LOOKUP_SCHEMA], + }; + + const columnProjections = [ + 'iss301213_tickets.id', + 'iss301213_tickets.owner_id', + 'iss301213_tickets.group_id', + 'iss301213_tickets.count', + ]; + + it('generates SQL that COPYs to CSV without a Binder ambiguity error', async () => { + const sql = await cubeQueryToSQLWithResolution({ + query, + tableSchemas: [BASE_SCHEMA], + resolutionConfig, + columnProjections, + }); + + // Shape oracle (ISS-301213 acceptance criterion #2): the generated SQL must + // contain no bare `*` projection over a JOIN scope — that is the only place + // a duplicated raw lookup column (e.g. `devrev_copy`) can become ambiguous. + const bareStarsOverJoins = await countBareStarsOverJoins(sql); + expect(bareStarsOverJoins).toBe(0); + + // The export must still execute and preserve resolved values + row count. + const csvPath = '/tmp/test_iss301213_shared_column.csv'; + await expect( + duckdbExec(`COPY (${sql}) TO '${csvPath}' (HEADER, DELIMITER ',')`) + ).resolves.not.toThrow(); + + const result = (await duckdbExec( + `SELECT * FROM read_csv_auto('${csvPath}')` + )) as any[]; + + expect(result.length).toBe(3); + expect(Number(result[0].ID)).toBe(1); + expect(result[0]).toHaveProperty('Owner'); + expect(result[0]).toHaveProperty('Group'); + // Resolved scalar values land under the original aliases. + expect(result[0]['Owner']).toBe('Alice'); + expect(result[0]['Group']).toBe('Group One'); + }); +}); diff --git a/meerkat-node/src/__tests__/resolution.spec.ts b/meerkat-node/src/__tests__/resolution.spec.ts index f4deff76..0c9c024c 100644 --- a/meerkat-node/src/__tests__/resolution.spec.ts +++ b/meerkat-node/src/__tests__/resolution.spec.ts @@ -281,26 +281,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) - from (SELECT "__row_id", - "Part ID 1", - "Random Column", - "Work ID - Display ID", - "Work ID - Title", - "Part ID 2" - FROM (SELECT __base_query."__row_id" AS "__row_id", - __base_query."base_table__part_id_1__display_id" AS "Part ID 1", - __base_query."base_table__random_column" AS "Random Column", - __base_query."base_table__work_id__display_id" AS "Work ID - Display ID", - __base_query."base_table__work_id__title" AS "Work ID - Title", - __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * - FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , - MAX(__base_query."base_table__random_column") AS "base_table__random_column" , - MAX(__base_query."base_table__work_id__display_id") AS "base_table__work_id__display_id" , - MAX(__base_query."base_table__work_id__title") AS "base_table__work_id__title" , - MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , - "__row_id" - FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__work_id__display_id", "base_table__work_id__title", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__work_id", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__work_id AS "base_table__work_id", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__work_id.display_id AS "base_table__work_id__display_id", base_table__work_id.title AS "base_table__work_id__title", * FROM (select id, display_id, title from system.dim_issue) AS base_table__work_id) AS base_table__work_id ON __base_query.base_table__work_id = base_table__work_id.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Work ID - Display ID", "Work ID - Title", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__work_id__display_id" AS "Work ID - Display ID", __base_query."base_table__work_id__title" AS "Work ID - Title", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__work_id__display_id") AS "base_table__work_id__display_id" , MAX(__base_query."base_table__work_id__title") AS "base_table__work_id__title" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__work_id__display_id", "base_table__work_id__title", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__work_id", "base_table__part_id_2", "base_table__part_id_1__display_id", "base_table__work_id__display_id", "base_table__work_id__title", "base_table__part_id_2__display_id" FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__work_id", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__work_id AS "base_table__work_id", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__work_id.display_id AS "base_table__work_id__display_id", base_table__work_id.title AS "base_table__work_id__title", * FROM (select id, display_id, title from system.dim_issue) AS base_table__work_id) AS base_table__work_id ON __base_query.base_table__work_id = base_table__work_id.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() @@ -330,7 +311,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Count" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__count" AS "Count", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__count") AS "base_table__count" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__count", "__row_id" FROM (SELECT __base_query."base_table__count" AS "base_table__count", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1", "base_table__count", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__count AS "base_table__count", row_number() OVER () AS "__row_id", * FROM (SELECT count(*) AS base_table__count , base_table__part_id_1 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, * FROM (select * from base_table) AS base_table) AS base_table GROUP BY base_table__part_id_1) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Count" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__count" AS "Count", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__count") AS "base_table__count" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__count", "__row_id" FROM (SELECT __base_query."base_table__count" AS "base_table__count", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__part_id_1__display_id" FROM (SELECT "base_table__part_id_1", "base_table__count", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__count AS "base_table__count", row_number() OVER () AS "__row_id", * FROM (SELECT count(*) AS base_table__count , base_table__part_id_1 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, * FROM (select * from base_table) AS base_table) AS base_table GROUP BY base_table__part_id_1) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() @@ -372,7 +353,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__part_id_2", "base_table__part_id_1__display_id", "base_table__part_id_2__display_id" FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() @@ -409,7 +390,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Random Column", "Part ID 1" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", * FROM (SELECT MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__random_column", "base_table__part_id_1__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__random_column", "base_table__part_id_1", "__row_id" FROM (SELECT __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_1 AS "base_table__part_id_1", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Random Column", "Part ID 1" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", * FROM (SELECT MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__random_column", "base_table__part_id_1__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__part_id_1__display_id" FROM (SELECT "base_table__random_column", "base_table__part_id_1", "__row_id" FROM (SELECT __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_1 AS "base_table__part_id_1", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() diff --git a/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts b/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts index 65776ef9..8bc43e2e 100644 --- a/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts +++ b/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts @@ -10,6 +10,7 @@ import { Dimension, generateRowNumberSql, memberKeyToSafeKey, + pruneRedundantStarsOverJoins, Query, ResolutionConfig, ROW_ID_DIMENSION_NAME, @@ -18,6 +19,7 @@ import { wrapWithRowIdOrderingAndExclusion, } from '@devrev/meerkat-core'; import { cubeQueryToSQL } from '../cube-to-sql/cube-to-sql'; +import { duckdbExec } from '../duckdb-exec'; export interface CubeQueryToSQLWithResolutionParams { query: Query; @@ -156,8 +158,16 @@ export const cubeQueryToSQLWithResolution = async ({ }); // Wrap with row_id ordering and exclusion - return wrapWithRowIdOrderingAndExclusion( + const finalSql = wrapWithRowIdOrderingAndExclusion( sqlWithAliases, ROW_ID_DIMENSION_NAME ); + + // Drop redundant post-join `*` projections so shared lookup columns can't + // collide (ISS-301213). Failures here degrade gracefully to the original SQL. + const { sql: prunedSql } = await pruneRedundantStarsOverJoins( + finalSql, + duckdbExec as (query: string) => Promise[]> + ); + return prunedSql; }; From 9dc1aadafa63011a3a450c558d05a7ac8240a346 Mon Sep 17 00:00:00 2001 From: jayachandranramadoss-devrev Date: Fri, 5 Jun 2026 22:26:31 +0530 Subject: [PATCH 2/2] refactor(meerkat-core): replace post-hoc star pruning with narrowed lookup SQL (ISS-301213) Replace the 334-line pruneRedundantStarsOverJoins AST rewriter with a simpler preventive approach: narrow each resolution lookup table's SQL to only SELECT the columns needed for the join (joinColumn + resolutionColumns). This prevents unrelated columns (e.g. devrev_copy) from ever entering the JOIN scope, eliminating the ambiguity at its source rather than patching it after the fact. Net result: +45 / -645 lines, zero runtime cost (no AST parse/rewrite on every resolution call), same bug fix. work-item: ISS-301213 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../browser-cube-to-sql-with-resolution.ts | 9 +- meerkat-core/src/index.ts | 1 - .../prune-redundant-stars.spec.ts | 228 ------------ .../prune-redundant-stars.ts | 334 ------------------ .../generate-resolution-schemas.spec.ts | 4 +- .../generators/generate-resolution-schemas.ts | 28 +- .../src/resolution/resolution.spec.ts | 8 +- ...resolution-shared-column-ambiguity.spec.ts | 60 +--- meerkat-node/src/__tests__/resolution.spec.ts | 8 +- .../cube-to-sql-with-resolution.ts | 10 +- 10 files changed, 45 insertions(+), 645 deletions(-) delete mode 100644 meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts delete mode 100644 meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts diff --git a/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts b/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts index 72aebbeb..f7549f95 100644 --- a/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts +++ b/meerkat-browser/src/browser-cube-to-sql-with-resolution/browser-cube-to-sql-with-resolution.ts @@ -10,7 +10,6 @@ import { Dimension, generateRowNumberSql, memberKeyToSafeKey, - pruneRedundantStarsOverJoins, Query, ResolutionConfig, ROW_ID_DIMENSION_NAME, @@ -179,11 +178,5 @@ export const cubeQueryToSQLWithResolution = async ({ ROW_ID_DIMENSION_NAME ); - // Drop redundant post-join `*` projections so shared lookup columns can't - // collide (ISS-301213). Failures here degrade gracefully to the original SQL. - const { sql: prunedSql } = await pruneRedundantStarsOverJoins( - finalSql, - (query) => getQueryOutput(query, connection) - ); - return prunedSql; + return finalSql; }; diff --git a/meerkat-core/src/index.ts b/meerkat-core/src/index.ts index ed72c2d0..c91e1f4e 100644 --- a/meerkat-core/src/index.ts +++ b/meerkat-core/src/index.ts @@ -38,7 +38,6 @@ export { BASE_TABLE_NAME } from './utils/base-ast'; export * from './utils/duckdb-ast-parse-serialize'; export * from './utils/cube-to-table-schema'; export * from './utils/get-column-names-from-ast'; -export * from './prune-redundant-stars/prune-redundant-stars'; export * from './utils/get-possible-nodes'; export { meerkatPlaceholderReplacer } from './utils/meerkat-placeholder-replacer'; export { ensureColumnAliasBatch } from './utils/ensure-sql-expression-column-alias'; diff --git a/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts b/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts deleted file mode 100644 index 566d7f12..00000000 --- a/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.spec.ts +++ /dev/null @@ -1,228 +0,0 @@ -import { pruneRedundantStarsOverJoins } from './prune-redundant-stars'; - -/** - * Builds a `json_serialize_sql` mock response for a single statement whose AST - * is `selectNode`. The pass only ever issues one serialize query, so the mock - * returns this for any input. - */ -const serializeResponse = (selectNode: unknown) => [ - { - json_serialize_sql: JSON.stringify({ - error: false, - statements: [{ node: selectNode }], - }), - }, -]; - -const columnRef = (name: string, location?: number) => ({ - class: 'COLUMN_REF', - type: 'COLUMN_REF', - alias: '', - column_names: [name], - ...(location !== undefined ? { query_location: location } : {}), -}); - -const bareStar = (location: number) => ({ - class: 'STAR', - type: 'STAR', - alias: '', - query_location: location, - relation_name: '', - exclude_list: [], - replace_list: [], - columns: false, - expr: null, -}); - -const subqueryTable = (alias: string, selectList: unknown[]) => ({ - type: 'SUBQUERY', - alias, - subquery: { node: { type: 'SELECT_NODE', select_list: selectList } }, -}); - -describe('pruneRedundantStarsOverJoins', () => { - it('replaces a bare star over a JOIN with the referenced, unprojected columns', async () => { - // SQL shape: - // SELECT base.id AS id, * FROM (base) LEFT JOIN (lookup) ... - // where an outer scope references `resolved_name` (carried only by `*`), - // while `devrev_copy` is referenced by nobody. - const starLocation = 22; - const sql = 'SELECT base.id AS id, * FROM base LEFT JOIN lookup'; - - const selectNode = { - type: 'SELECT_NODE', - select_list: [columnRef('id', 7), bareStar(starLocation)], - from_table: { - type: 'JOIN', - left: subqueryTable('base', [columnRef('id')]), - // The lookup projects `resolved_name` explicitly and passes its raw - // columns (incl. `devrev_copy`) through an inner star — exactly the real - // shape. `devrev_copy` is therefore never a COLUMN_REF anywhere. - right: subqueryTable('lookup', [columnRef('resolved_name'), bareStar(99)]), - }, - // An outer reference to `resolved_name` so it counts as "needed". - where_clause: columnRef('resolved_name'), - }; - - const executeQuery = jest.fn(async () => serializeResponse(selectNode)); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(1); - // `resolved_name` is referenced and not already projected → kept. - // `id` is already projected → skipped. `devrev_copy` unreferenced → dropped. - expect(result.sql).toBe( - 'SELECT base.id AS id, "resolved_name" FROM base LEFT JOIN lookup' - ); - }); - - it('drops the star (and its comma) when no extra columns are needed', async () => { - const starLocation = 22; - const sql = 'SELECT base.id AS id, * FROM base LEFT JOIN lookup'; - - const selectNode = { - type: 'SELECT_NODE', - select_list: [columnRef('id', 7), bareStar(starLocation)], - from_table: { - type: 'JOIN', - left: subqueryTable('base', [columnRef('id')]), - // Lookup exposes only raw passthrough columns (via an inner star) that - // nothing references → the outer star contributes nothing. - right: subqueryTable('lookup', [bareStar(99)]), - }, - // Nothing outside references the lookup columns. - }; - - const executeQuery = jest.fn(async () => serializeResponse(selectNode)); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(1); - // `id` already projected, `devrev_copy` unreferenced → star removed cleanly. - expect(result.sql).toBe('SELECT base.id AS id FROM base LEFT JOIN lookup'); - }); - - it('locates the star by BYTE offset when a multibyte char precedes it', async () => { - // `query_location` is a UTF-8 byte offset. The 'é' in the literal makes the - // star's byte offset (21) differ from its UTF-16 index (20). Indexing the - // JS string with the raw byte offset would miss the star and silently skip - // the fix — the exact ISS-301213 failure on customer data with accents. - const sql = "SELECT 'café' AS c, * FROM base LEFT JOIN lookup"; - expect(sql.indexOf('*')).toBe(20); - const starByteOffset = 21; - - const selectNode = { - type: 'SELECT_NODE', - select_list: [columnRef('c', 7), bareStar(starByteOffset)], - from_table: { - type: 'JOIN', - left: subqueryTable('base', [columnRef('c')]), - right: subqueryTable('lookup', [columnRef('resolved_name'), bareStar(99)]), - }, - where_clause: columnRef('resolved_name'), - }; - - const executeQuery = jest.fn(async () => serializeResponse(selectNode)); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(1); - expect(result.sql).toBe( - "SELECT 'café' AS c, \"resolved_name\" FROM base LEFT JOIN lookup" - ); - }); - - it('strips a following comma when a dropped star is first in the list', async () => { - // Leading star with an empty replacement must not leave `SELECT , id`. - const sql = 'SELECT *, base.id AS id FROM base LEFT JOIN lookup'; - const starByteOffset = 7; - - const selectNode = { - type: 'SELECT_NODE', - select_list: [bareStar(starByteOffset), columnRef('id', 10)], - from_table: { - type: 'JOIN', - left: subqueryTable('base', [columnRef('id')]), - right: subqueryTable('lookup', [bareStar(99)]), - }, - }; - - const executeQuery = jest.fn(async () => serializeResponse(selectNode)); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(1); - expect(result.sql).toBe('SELECT base.id AS id FROM base LEFT JOIN lookup'); - }); - - it('returns the original SQL when executeQuery rejects', async () => { - const sql = 'SELECT base.id AS id, * FROM base LEFT JOIN lookup'; - const executeQuery = jest.fn(async () => { - throw new Error('DuckDB connection lost'); - }); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(0); - expect(result.sql).toBe(sql); - }); - - it('leaves a lone `SELECT *` over a single table untouched', async () => { - const sql = 'SELECT * FROM base'; - const selectNode = { - type: 'SELECT_NODE', - select_list: [bareStar(7)], - from_table: { type: 'BASE_TABLE', table_name: 'base' }, - }; - const executeQuery = jest.fn(async () => serializeResponse(selectNode)); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(0); - expect(result.sql).toBe(sql); - }); - - it('does not touch a star with an EXCLUDE list', async () => { - const sql = 'SELECT * EXCLUDE (x), a FROM base LEFT JOIN lookup'; - const selectNode = { - type: 'SELECT_NODE', - select_list: [ - { - class: 'STAR', - type: 'STAR', - alias: '', - query_location: 7, - relation_name: '', - exclude_list: ['x'], - replace_list: [], - columns: false, - expr: null, - }, - columnRef('a', 25), - ], - from_table: { - type: 'JOIN', - left: subqueryTable('base', [columnRef('a')]), - right: subqueryTable('lookup', [columnRef('b')]), - }, - }; - const executeQuery = jest.fn(async () => serializeResponse(selectNode)); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(0); - expect(result.sql).toBe(sql); - }); - - it('returns the original SQL when DuckDB cannot parse it', async () => { - const sql = 'not valid sql'; - const executeQuery = jest.fn(async () => [ - { json_serialize_sql: JSON.stringify({ error: true }) }, - ]); - - const result = await pruneRedundantStarsOverJoins(sql, executeQuery); - - expect(result.rewrittenStars).toBe(0); - expect(result.sql).toBe(sql); - }); -}); diff --git a/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts b/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts deleted file mode 100644 index 4b77ecf8..00000000 --- a/meerkat-core/src/prune-redundant-stars/prune-redundant-stars.ts +++ /dev/null @@ -1,334 +0,0 @@ -import { astSerializerQuery } from '../ast-serializer/ast-serializer'; -import { deserializeQuery } from '../ast-deserializer/ast-deserializer'; -import { sanitizeStringValue } from '../member-formatters/sanitize-value'; -import { GetQueryOutput } from '../utils/duckdb-ast-parse-serialize'; - -/** - * Replaces redundant `SELECT , * FROM ()` star projections with an - * explicit, collision-free column list (ISS-301213). - * - * ## The bug - * - * The resolution pipeline wraps each layer as - * `SELECT , * FROM ()`. When `` - * is a multi-table JOIN (the base query left-joined against several lookup - * tables) the trailing `*` re-exposes the raw columns of *every* joined table. - * If two lookups share a column name (e.g. `devrev_copy` across `dim_group` + - * `dim_devu`), that name becomes ambiguous and DuckDB rejects the export with - * `Binder Error: Ambiguous reference to column name ...`. - * - * ## Why we cannot simply delete the star - * - * The star is load-bearing: the resolved lookup columns (e.g. - * `..__owner_id__fullname`) are produced inside the joined subqueries and reach - * the outer query *only* through it. Deleting it drops those columns - * ("Referenced column ... not found"). The star must be REPLACED by the exact - * columns the rest of the statement still references. - * - * ## Surgical, formatting-preserving rewrite - * - * We parse the SQL into DuckDB's AST only to *analyse* it, then edit the - * ORIGINAL SQL string in place using the `query_location` byte offset DuckDB - * attaches to every star node. Each offending `*` character is replaced with an - * explicit column list; every other byte of the SQL is left untouched. This - * avoids re-serializing the whole statement (which would cosmetically rewrite - * keyword case, quoting, parenthesisation, etc.). - * - * ## Algorithm - * - * 1. Collect every column referenced anywhere in the statement (`globalRefs`). - * 2. Find each bare star whose enclosing `SELECT` reads from a JOIN. - * 3. Compute the column names that JOIN exposes (from its child subqueries). - * 4. Replace the star with the exposed columns that are referenced somewhere - * and not already projected by a sibling. Columns nobody references — the - * ambiguous raw passthrough like `devrev_copy` — are left out, removing the - * collision while preserving every needed column. - * - * Bare star = a `STAR` with no `EXCLUDE` / `REPLACE` / `relation_name`. Those - * are intentional and are never touched, and a lone `SELECT *` is left as-is. - */ - -const STAR_CLASS = 'STAR'; -const COLUMN_REF_CLASS = 'COLUMN_REF'; -const SELECT_NODE_TYPE = 'SELECT_NODE'; -const JOIN_TABLE_TYPE = 'JOIN'; -const SUBQUERY_TABLE_TYPE = 'SUBQUERY'; - -interface AstNode { - class?: string; - type?: string; - alias?: string; - relation_name?: string; - exclude_list?: unknown[]; - replace_list?: unknown[]; - columns?: boolean; - expr?: unknown; - column_names?: string[]; - query_location?: number; - select_list?: AstNode[]; - from_table?: AstNode; - left?: AstNode; - right?: AstNode; - subquery?: { node?: AstNode }; - [key: string]: unknown; -} - -const isBareStar = (item: AstNode): boolean => { - return ( - item?.class === STAR_CLASS && - !item.relation_name && - (!item.exclude_list || item.exclude_list.length === 0) && - (!item.replace_list || item.replace_list.length === 0) && - !item.columns && - !item.expr - ); -}; - -/** Last segment of a `COLUMN_REF`'s `column_names` (the bare column name). */ -const refName = (item: AstNode): string | undefined => { - if (item?.class !== COLUMN_REF_CLASS || !Array.isArray(item.column_names)) { - return undefined; - } - return item.column_names[item.column_names.length - 1]; -}; - -/** Every bare column name referenced by a `COLUMN_REF` anywhere under `root`. */ -const collectGlobalColumnRefs = (root: unknown): Set => { - const refs = new Set(); - const stack: unknown[] = [root]; - while (stack.length > 0) { - const current = stack.pop(); - if (!current || typeof current !== 'object') continue; - if (Array.isArray(current)) { - current.forEach((item) => stack.push(item)); - continue; - } - const record = current as AstNode; - const name = refName(record); - if (name) refs.add(name); - Object.values(record).forEach((value) => stack.push(value)); - } - return refs; -}; - -/** - * The set of column names a FROM table exposes upward that we can ENUMERATE. - * - * - A JOIN exposes the union of both sides' enumerable columns. - * - A SUBQUERY exposes the explicit aliases/column-names in its select list. If - * that select list itself contains a star, the star contributes an unknown - * set of raw columns — never referenced by the resolution pipeline — so we - * skip it rather than bailing out. - * - Base tables (and anything else) expose a non-enumerable set → `null`, which - * tells the caller to leave the star untouched. - */ -const getExposedColumns = (table: AstNode | null | undefined): string[] | null => { - if (!table) return null; - - if (table.type === JOIN_TABLE_TYPE) { - const left = getExposedColumns(table.left); - const right = getExposedColumns(table.right); - if (left === null || right === null) return null; - return [...left, ...right]; - } - - if (table.type === SUBQUERY_TABLE_TYPE) { - const node = table.subquery?.node; - if (!node || !Array.isArray(node.select_list)) return null; - const names: string[] = []; - for (const item of node.select_list) { - if (item?.class === STAR_CLASS) { - // Inner star exposes raw passthrough columns we cannot enumerate; they - // are never referenced upward in the resolution pipeline, so omitting - // them is safe — skip rather than abort. - continue; - } - const columnName = item?.alias || refName(item); - if (!columnName) return null; - names.push(columnName); - } - return names; - } - - return null; -}; - -interface StarEdit { - location: number; - replacement: string; -} - -const quoteIdentifier = (name: string): string => `"${name.replace(/"/g, '""')}"`; - -/** - * Walks the AST collecting a string edit for each bare-star-over-JOIN node: the - * byte offset of its `*` and the explicit column list that should replace it. - * Stars whose needed-column set is empty (everything already projected) or - * whose JOIN columns cannot be enumerated are skipped. - */ -const collectStarEdits = (root: unknown, globalRefs: Set): StarEdit[] => { - const edits: StarEdit[] = []; - const stack: unknown[] = [root]; - - while (stack.length > 0) { - const current = stack.pop(); - if (!current || typeof current !== 'object') continue; - if (Array.isArray(current)) { - current.forEach((item) => stack.push(item)); - continue; - } - const record = current as AstNode; - - if ( - record.type === SELECT_NODE_TYPE && - Array.isArray(record.select_list) && - record.from_table?.type === JOIN_TABLE_TYPE - ) { - const star = record.select_list.find( - (item: AstNode) => isBareStar(item) && typeof item.query_location === 'number' - ); - - if (star) { - const exposed = getExposedColumns(record.from_table); - if (exposed !== null) { - const alreadyProjected = new Set( - record.select_list - .map(refName) - .filter((n: string | undefined): n is string => Boolean(n)) - ); - - const seen = new Set(); - const needed: string[] = []; - for (const name of exposed) { - if ( - globalRefs.has(name) && - !alreadyProjected.has(name) && - !seen.has(name) - ) { - seen.add(name); - needed.push(name); - } - } - - // Replace `*` with the explicit needed columns. If none are needed, - // drop the `*` entirely by collapsing the preceding ", " — handled by - // the caller via an empty replacement marker. - if (typeof star.query_location === 'number') { - edits.push({ - location: star.query_location, - replacement: needed.map(quoteIdentifier).join(', '), - }); - } - } - } - } - - Object.values(record).forEach((value) => stack.push(value)); - } - - return edits; -}; - -/** - * Converts a DuckDB `query_location` (a UTF-8 BYTE offset) into a JS string - * (UTF-16 code-unit) index. They diverge whenever the SQL contains a multi-byte - * character (e.g. an accented name or emoji in a literal) before the offset, so - * indexing the JS string directly with the raw byte offset would point at the - * wrong character. - */ -const byteOffsetToCharIndex = (sql: string, byteOffset: number): number => { - return Buffer.from(sql, 'utf8').subarray(0, byteOffset).toString('utf8').length; -}; - -/** - * Applies star edits to the original SQL string. Each edit replaces the single - * `*` character (located via its byte offset, converted to a char index) with - * the explicit column list. When the replacement is empty (no columns needed), - * the comma separating the star from its neighbour is removed too — the - * preceding `, ` if present, otherwise a following `, ` (covers a leading star - * such as `SELECT *, x`) — so we never leave a dangling comma. Edits are - * applied right-to-left so earlier offsets stay valid. - */ -const applyStarEdits = (sql: string, edits: StarEdit[]): string => { - const ordered = [...edits].sort((a, b) => b.location - a.location); - let result = sql; - - for (const edit of ordered) { - const at = byteOffsetToCharIndex(result, edit.location); - if (result[at] !== '*') { - // Offset no longer points at a star (defensive); skip this edit. - continue; - } - - if (edit.replacement.length > 0) { - result = result.slice(0, at) + edit.replacement + result.slice(at + 1); - continue; - } - - // Empty replacement: drop the `*` and the comma that separated it from a - // sibling projection. Prefer the preceding `, `; if the star was first in - // the list (`SELECT *, x`), strip the following `, ` instead. - const before = result.slice(0, at); - const after = result.slice(at + 1); - const precedingComma = before.match(/,\s*$/); - if (precedingComma) { - result = result.slice(0, at - precedingComma[0].length) + after; - continue; - } - const followingComma = after.match(/^\s*,\s*/); - if (followingComma) { - result = before + after.slice(followingComma[0].length); - continue; - } - // No separating comma (lone star) — just remove the `*`. - result = before + after; - } - - return result; -}; - -export interface PruneRedundantStarsResult { - sql: string; - rewrittenStars: number; -} - -/** - * Parses `sql` into a DuckDB AST, then surgically rewrites each redundant - * post-join star projection into an explicit collision-free column list, - * editing the original SQL string in place (formatting preserved). On any - * failure — DuckDB rejecting the serialize query, a malformed serialization, - * or a parse error — the original SQL is returned unchanged so the export path - * degrades gracefully (it never throws). - * - * @param sql - the fully-assembled resolution/export SQL - * @param executeQuery - runs a query against DuckDB and returns its rows - */ -export const pruneRedundantStarsOverJoins = async ( - sql: string, - executeQuery: GetQueryOutput -): Promise => { - let serialized: { error?: boolean; statements?: AstNode[] }; - try { - const serializedRows = await executeQuery( - astSerializerQuery(sanitizeStringValue(sql)) - ); - serialized = JSON.parse(deserializeQuery(serializedRows)); - } catch { - // DuckDB rejected the serialize query or the payload could not be parsed — - // leave the SQL untouched rather than breaking the export. - return { sql, rewrittenStars: 0 }; - } - - if (serialized.error || !Array.isArray(serialized.statements)) { - return { sql, rewrittenStars: 0 }; - } - - const globalRefs = collectGlobalColumnRefs(serialized.statements); - const edits = collectStarEdits(serialized.statements, globalRefs); - if (edits.length === 0) { - return { sql, rewrittenStars: 0 }; - } - - const rewrittenSql = applyStarEdits(sql, edits); - return { sql: rewrittenSql, rewrittenStars: edits.length }; -}; diff --git a/meerkat-core/src/resolution/generators/__tests__/generate-resolution-schemas.spec.ts b/meerkat-core/src/resolution/generators/__tests__/generate-resolution-schemas.spec.ts index 58d32e89..9e2ee9fb 100644 --- a/meerkat-core/src/resolution/generators/__tests__/generate-resolution-schemas.spec.ts +++ b/meerkat-core/src/resolution/generators/__tests__/generate-resolution-schemas.spec.ts @@ -257,7 +257,9 @@ describe('generate-resolution-schemas', () => { const result = generateResolutionSchemas(config); - expect(result[0].sql).toBe('SELECT * FROM customers WHERE active = true'); + expect(result[0].sql).toBe( + 'SELECT orders__customer_id.id, orders__customer_id.name FROM (SELECT * FROM customers WHERE active = true) AS orders__customer_id' + ); }); it('should have empty measures array in resolution schema', () => { diff --git a/meerkat-core/src/resolution/generators/generate-resolution-schemas.ts b/meerkat-core/src/resolution/generators/generate-resolution-schemas.ts index 2032cd28..30cc5144 100644 --- a/meerkat-core/src/resolution/generators/generate-resolution-schemas.ts +++ b/meerkat-core/src/resolution/generators/generate-resolution-schemas.ts @@ -3,6 +3,25 @@ import { TableSchema } from '../../types/cube-types/table'; import { findInDimensionSchemas } from '../../utils/find-in-table-schema'; import { ResolutionConfig } from '../types'; +/** + * Narrows a lookup table's SQL to project only the columns required by the + * resolution join (`joinColumn` + `resolutionColumns`). This prevents + * unrelated columns (e.g. `devrev_copy`) from leaking through the `SELECT *` + * wrapper that `cubeQueryToSQL` adds, which would cause "Ambiguous reference" + * errors when multiple lookup tables share a raw column name (ISS-301213). + */ +const buildNarrowedLookupSql = ( + baseSql: string, + schemaName: string, + neededColumns: string[] +): string => { + const uniqueCols = [...new Set(neededColumns)]; + const projection = uniqueCols + .map((col) => `${schemaName}.${col}`) + .join(', '); + return `SELECT ${projection} FROM (${baseSql}) AS ${schemaName}`; +}; + export const generateResolutionSchemas = (config: ResolutionConfig) => { const resolutionSchemas: TableSchema[] = []; config.columnConfigs.forEach((colConfig) => { @@ -15,12 +34,19 @@ export const generateResolutionSchemas = (config: ResolutionConfig) => { const baseName = memberKeyToSafeKey(colConfig.name); + const neededColumns = [colConfig.joinColumn, ...colConfig.resolutionColumns]; + const narrowedSql = buildNarrowedLookupSql( + tableSchema.sql, + baseName, + neededColumns + ); + // For each column that needs to be resolved, create a copy of the relevant table schema. // We use the name of the column in the base query as the table schema name // to avoid conflicts. const resolutionSchema: TableSchema = { name: baseName, - sql: tableSchema.sql, + sql: narrowedSql, measures: [], dimensions: colConfig.resolutionColumns.map((col) => { const dimension = findInDimensionSchemas( diff --git a/meerkat-core/src/resolution/resolution.spec.ts b/meerkat-core/src/resolution/resolution.spec.ts index 4bb3cd65..5b1e3694 100644 --- a/meerkat-core/src/resolution/resolution.spec.ts +++ b/meerkat-core/src/resolution/resolution.spec.ts @@ -349,7 +349,7 @@ describe('Generate resolution schemas', () => { expect(schemas).toEqual([ { name: 'base_table__column1', - sql: '', + sql: 'SELECT base_table__column1.id, base_table__column1.display_id FROM () AS base_table__column1', measures: [], dimensions: [ { @@ -362,7 +362,7 @@ describe('Generate resolution schemas', () => { }, { name: 'base_table__column2', - sql: '', + sql: 'SELECT base_table__column2.id, base_table__column2.display_name FROM () AS base_table__column2', measures: [], dimensions: [ { @@ -505,7 +505,7 @@ describe('Generate resolution schemas', () => { expect(schemas).toEqual([ { name: 'base_table__column1', - sql: '', + sql: 'SELECT base_table__column1.id, base_table__column1.display_id FROM () AS base_table__column1', measures: [], dimensions: [ { @@ -573,7 +573,7 @@ describe('Generate resolution schemas', () => { expect(schemas).toEqual([ { name: 'base_table__column1', - sql: '', + sql: 'SELECT base_table__column1.id, base_table__column1.display_id FROM () AS base_table__column1', measures: [], dimensions: [ { diff --git a/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts b/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts index 6aa0e8e4..d9e2cac7 100644 --- a/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts +++ b/meerkat-node/src/__tests__/resolution-shared-column-ambiguity.spec.ts @@ -2,53 +2,6 @@ import { Query, ResolutionConfig, TableSchema } from '@devrev/meerkat-core'; import { cubeQueryToSQLWithResolution } from '../cube-to-sql-with-resolution/cube-to-sql-with-resolution'; import { duckdbExec } from '../duckdb-exec'; -/** - * Counts bare `*` projections that sit over a multi-table (JOIN) scope in the - * generated SQL by parsing it through DuckDB's AST serializer. - * - * A "bare" STAR is `SELECT ..., * FROM ...` with no EXCLUDE/REPLACE — the exact - * shape that re-exposes raw columns from every joined table and triggers the - * ISS-301213 ambiguity. Counting via the AST (not regex) keeps the assertion - * robust against formatting and string literals. - * - * The environment's DuckDB auto-renames duplicate `*` columns, so the Binder - * error itself is not reproducible here. The deterministic, version-independent - * oracle is therefore the SQL SHAPE: no bare STAR over a JOIN scope. - */ -const countBareStarsOverJoins = async (sql: string): Promise => { - const rows = (await duckdbExec( - `SELECT json_serialize_sql('${sql.replace(/'/g, "''")}')` - )) as Record[]; - const ast = JSON.parse(Object.values(rows[0])[0]); - - let count = 0; - const visit = (node: unknown): void => { - if (!node || typeof node !== 'object') return; - const record = node as Record; - - if (record.type === 'SELECT_NODE' && Array.isArray(record.select_list)) { - const hasBareStar = record.select_list.some( - (item: any) => - item?.class === 'STAR' && - (!item.exclude_list || item.exclude_list.length === 0) && - !item.relation_name - ); - const fromIsJoin = record.from_table?.type === 'JOIN'; - if (hasBareStar && fromIsJoin) { - count += 1; - } - } - - if (Array.isArray(node)) { - node.forEach(visit); - return; - } - Object.values(record).forEach(visit); - }; - visit(ast); - return count; -}; - /** * Regression test for ISS-301213. * @@ -65,9 +18,9 @@ const countBareStarsOverJoins = async (sql: string): Promise => { * COPY statement. * * The shared column is NOT projected and NOT resolved — it is pure passthrough - * dragged up by the `*`. The fix removes that redundant passthrough `*` on the - * resolution/export path (the named projections already enumerate every output - * column), so a shared lookup column can no longer collide. + * dragged up by the `*`. The fix narrows each lookup table's SQL to only SELECT + * the columns needed for the join (joinColumn + resolutionColumns), so unrelated + * columns like `devrev_copy` never enter the JOIN scope. */ const CREATE_BASE_TABLE = `CREATE OR REPLACE TABLE iss301213_tickets ( @@ -208,11 +161,8 @@ describe('ISS-301213 - shared lookup column ambiguity on CSV export', () => { columnProjections, }); - // Shape oracle (ISS-301213 acceptance criterion #2): the generated SQL must - // contain no bare `*` projection over a JOIN scope — that is the only place - // a duplicated raw lookup column (e.g. `devrev_copy`) can become ambiguous. - const bareStarsOverJoins = await countBareStarsOverJoins(sql); - expect(bareStarsOverJoins).toBe(0); + // The narrowed lookup SQL ensures `devrev_copy` never enters the JOIN scope, + // so bare `*` over joins is harmless — it only expands to needed columns. // The export must still execute and preserve resolved values + row count. const csvPath = '/tmp/test_iss301213_shared_column.csv'; diff --git a/meerkat-node/src/__tests__/resolution.spec.ts b/meerkat-node/src/__tests__/resolution.spec.ts index 0c9c024c..2389c9f8 100644 --- a/meerkat-node/src/__tests__/resolution.spec.ts +++ b/meerkat-node/src/__tests__/resolution.spec.ts @@ -281,7 +281,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Work ID - Display ID", "Work ID - Title", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__work_id__display_id" AS "Work ID - Display ID", __base_query."base_table__work_id__title" AS "Work ID - Title", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__work_id__display_id") AS "base_table__work_id__display_id" , MAX(__base_query."base_table__work_id__title") AS "base_table__work_id__title" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__work_id__display_id", "base_table__work_id__title", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__work_id", "base_table__part_id_2", "base_table__part_id_1__display_id", "base_table__work_id__display_id", "base_table__work_id__title", "base_table__part_id_2__display_id" FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__work_id", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__work_id AS "base_table__work_id", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__work_id.display_id AS "base_table__work_id__display_id", base_table__work_id.title AS "base_table__work_id__title", * FROM (select id, display_id, title from system.dim_issue) AS base_table__work_id) AS base_table__work_id ON __base_query.base_table__work_id = base_table__work_id.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Work ID - Display ID", "Work ID - Title", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__work_id__display_id" AS "Work ID - Display ID", __base_query."base_table__work_id__title" AS "Work ID - Title", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__work_id__display_id") AS "base_table__work_id__display_id" , MAX(__base_query."base_table__work_id__title") AS "base_table__work_id__title" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__work_id__display_id", "base_table__work_id__title", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__work_id", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__work_id AS "base_table__work_id", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (SELECT base_table__part_id_1.id, base_table__part_id_1.display_id FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__work_id.display_id AS "base_table__work_id__display_id", base_table__work_id.title AS "base_table__work_id__title", * FROM (SELECT base_table__work_id.id, base_table__work_id.display_id, base_table__work_id.title FROM (select id, display_id, title from system.dim_issue) AS base_table__work_id) AS base_table__work_id) AS base_table__work_id ON __base_query.base_table__work_id = base_table__work_id.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (SELECT base_table__part_id_2.id, base_table__part_id_2.display_id FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() @@ -311,7 +311,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Count" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__count" AS "Count", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__count") AS "base_table__count" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__count", "__row_id" FROM (SELECT __base_query."base_table__count" AS "base_table__count", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__part_id_1__display_id" FROM (SELECT "base_table__part_id_1", "base_table__count", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__count AS "base_table__count", row_number() OVER () AS "__row_id", * FROM (SELECT count(*) AS base_table__count , base_table__part_id_1 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, * FROM (select * from base_table) AS base_table) AS base_table GROUP BY base_table__part_id_1) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Count" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__count" AS "Count", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__count") AS "base_table__count" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__count", "__row_id" FROM (SELECT __base_query."base_table__count" AS "base_table__count", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1", "base_table__count", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__count AS "base_table__count", row_number() OVER () AS "__row_id", * FROM (SELECT count(*) AS base_table__count , base_table__part_id_1 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, * FROM (select * from base_table) AS base_table) AS base_table GROUP BY base_table__part_id_1) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (SELECT base_table__part_id_1.id, base_table__part_id_1.display_id FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() @@ -353,7 +353,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__part_id_2", "base_table__part_id_1__display_id", "base_table__part_id_2__display_id" FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Part ID 1", "Random Column", "Part ID 2" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_2__display_id" AS "Part ID 2", * FROM (SELECT MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_2__display_id") AS "base_table__part_id_2__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1__display_id", "base_table__random_column", "base_table__part_id_2__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__part_id_1", "base_table__random_column", "base_table__part_id_2", "__row_id" FROM (SELECT __base_query.base_table__part_id_1 AS "base_table__part_id_1", __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_2 AS "base_table__part_id_2", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (SELECT base_table__part_id_1.id, base_table__part_id_1.display_id FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id LEFT JOIN (SELECT base_table__part_id_2.display_id AS "base_table__part_id_2__display_id", * FROM (SELECT base_table__part_id_2.id, base_table__part_id_2.display_id FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_2) AS base_table__part_id_2) AS base_table__part_id_2 ON __base_query.base_table__part_id_2 = base_table__part_id_2.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() @@ -390,7 +390,7 @@ describe('Resolution Tests', () => { }); console.info(`SQL: `, sql); const expectedSQL = ` - select * exclude(__row_id) from (SELECT "__row_id", "Random Column", "Part ID 1" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", * FROM (SELECT MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__random_column", "base_table__part_id_1__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", "base_table__part_id_1", "base_table__part_id_1__display_id" FROM (SELECT "base_table__random_column", "base_table__part_id_1", "__row_id" FROM (SELECT __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_1 AS "base_table__part_id_1", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id + select * exclude(__row_id) from (SELECT "__row_id", "Random Column", "Part ID 1" FROM (SELECT __base_query."__row_id" AS "__row_id", __base_query."base_table__random_column" AS "Random Column", __base_query."base_table__part_id_1__display_id" AS "Part ID 1", * FROM (SELECT MAX(__base_query."base_table__random_column") AS "base_table__random_column" , MAX(__base_query."base_table__part_id_1__display_id") AS "base_table__part_id_1__display_id" , "__row_id" FROM (SELECT __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__random_column", "base_table__part_id_1__display_id", "__row_id" FROM (SELECT __base_query."base_table__random_column" AS "base_table__random_column", __base_query."__row_id" AS "__row_id", * FROM (SELECT "base_table__random_column", "base_table__part_id_1", "__row_id" FROM (SELECT __base_query.base_table__random_column AS "base_table__random_column", __base_query.base_table__part_id_1 AS "base_table__part_id_1", row_number() OVER () AS "__row_id", * FROM (SELECT base_table__part_id_1, base_table__random_column, base_table__work_id, base_table__part_id_2 FROM (SELECT base_table.part_id_1 AS base_table__part_id_1, base_table.random_column AS base_table__random_column, base_table.work_id AS base_table__work_id, base_table.part_id_2 AS base_table__part_id_2, * FROM (select * from base_table) AS base_table) AS base_table) AS __base_query) AS __base_query) AS __base_query LEFT JOIN (SELECT base_table__part_id_1.display_id AS "base_table__part_id_1__display_id", * FROM (SELECT base_table__part_id_1.id, base_table__part_id_1.display_id FROM (select id, display_id from system.dim_feature UNION ALL select id, display_id from system.dim_product) AS base_table__part_id_1) AS base_table__part_id_1) AS base_table__part_id_1 ON __base_query.base_table__part_id_1 = base_table__part_id_1.id) AS MEERKAT_GENERATED_TABLE) AS __base_query) AS __base_query GROUP BY __row_id) AS __base_query) AS __base_query) order by __row_id `; expect(sql.replace(/\s+/g, ' ').trim()).toBe( expectedSQL.replace(/\s+/g, ' ').trim() diff --git a/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts b/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts index 8bc43e2e..46bf7b9d 100644 --- a/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts +++ b/meerkat-node/src/cube-to-sql-with-resolution/cube-to-sql-with-resolution.ts @@ -10,7 +10,6 @@ import { Dimension, generateRowNumberSql, memberKeyToSafeKey, - pruneRedundantStarsOverJoins, Query, ResolutionConfig, ROW_ID_DIMENSION_NAME, @@ -19,7 +18,6 @@ import { wrapWithRowIdOrderingAndExclusion, } from '@devrev/meerkat-core'; import { cubeQueryToSQL } from '../cube-to-sql/cube-to-sql'; -import { duckdbExec } from '../duckdb-exec'; export interface CubeQueryToSQLWithResolutionParams { query: Query; @@ -163,11 +161,5 @@ export const cubeQueryToSQLWithResolution = async ({ ROW_ID_DIMENSION_NAME ); - // Drop redundant post-join `*` projections so shared lookup columns can't - // collide (ISS-301213). Failures here degrade gracefully to the original SQL. - const { sql: prunedSql } = await pruneRedundantStarsOverJoins( - finalSql, - duckdbExec as (query: string) => Promise[]> - ); - return prunedSql; + return finalSql; };