From 13ba8393a61ba59e422beeca1a380dfa0c9b0b5b Mon Sep 17 00:00:00 2001 From: Johnson Abraham Date: Sat, 19 Sep 2026 12:23:36 +0530 Subject: [PATCH 1/5] fix(CEMAS-103): stop leading comma when a dimension fails to resolve addDimensionToSQLProjection gated the separator on loop index, so a dimension that isn't registered in tableSchemas (e.g. a hidden or stale field pulled in by an upstream caller) got skipped via continue but still left the next resolvable dimension with a leading comma, breaking the generated SQL with a DuckDB parser error. Gate on whether a dimension was actually emitted instead. work-item: CEMAS-103 --- .../cube-measure-transformer.spec.ts | 7 +++++++ .../cube-measure-transformer/cube-measure-transformer.ts | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.spec.ts b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.spec.ts index 0fd699a3..e4c22181 100644 --- a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.spec.ts +++ b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.spec.ts @@ -120,6 +120,13 @@ describe('cubeMeasureToSQLSelectString', () => { ); }); + it('should not emit a leading comma when an earlier dimension fails to resolve', () => { + const dimensions: Member[] = ['temp.unknown_dimension', 'temp.dimension1']; + const sqlToReplace = 'SELECT * FROM (SELECT * FROM TABLE_1)'; + const result = applyProjectionToSQLQuery(dimensions, [], [tableSchema], sqlToReplace); + expect(result).toBe(`SELECT temp__dimension1 FROM (SELECT * FROM TABLE_1)`); + }); + it('should use aliases when provided', () => { const measures: Member[] = ['temp.measure1', 'temp.measure2']; const sqlToReplace = 'SELECT * FROM my_table'; diff --git a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts index 460ec812..de4cc212 100644 --- a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts +++ b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts @@ -102,6 +102,7 @@ const addDimensionToSQLProjection = ( return selectString; } let newSelectString = selectString; + let hasEmittedDimension = false; for (let i = 0; i < dimensions.length; i++) { const dimension = dimensions[i]; const resolved = findSchemaForMember(dimension, tableSchemas); @@ -122,11 +123,13 @@ const addDimensionToSQLProjection = ( if (!dimensionSchema) { continue; } - if (i > 0) { + // Gate on emitted count, not loop index, so a skipped unresolved dimension doesn't leave a leading comma. + if (hasEmittedDimension) { newSelectString += ','; } // since alias key is expected to have been unfurled in the base query, we can just use it as is. newSelectString += ` ${aliasKey}`; + hasEmittedDimension = true; } return newSelectString; }; From 213aecc3e5022b3e4dfa53abc9e7b159ff23e5c0 Mon Sep 17 00:00:00 2001 From: Johnson Abraham Date: Mon, 21 Sep 2026 17:59:56 +0530 Subject: [PATCH 2/5] fix(CEMAS-103): clarify comment and bump package versions Address review feedback: - Bump meerkat-core, meerkat-browser, meerkat-node to 0.0.136. - Clarify why a dimension can go unemitted: `dimensions` is caller-supplied and not guaranteed to match `tableSchemas` (e.g. a caller passing a field deliberately excluded from the schema). work-item: CEMAS-103 --- meerkat-browser/package.json | 2 +- meerkat-core/package.json | 2 +- .../cube-measure-transformer/cube-measure-transformer.ts | 7 ++++++- meerkat-node/package.json | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/meerkat-browser/package.json b/meerkat-browser/package.json index e8d72931..d69a2e51 100644 --- a/meerkat-browser/package.json +++ b/meerkat-browser/package.json @@ -1,6 +1,6 @@ { "name": "@devrev/meerkat-browser", - "version": "0.0.135", + "version": "0.0.136", "dependencies": { "tslib": "^2.3.0", "@devrev/meerkat-core": "*", diff --git a/meerkat-core/package.json b/meerkat-core/package.json index 7d9c217d..44f087ce 100644 --- a/meerkat-core/package.json +++ b/meerkat-core/package.json @@ -1,6 +1,6 @@ { "name": "@devrev/meerkat-core", - "version": "0.0.135", + "version": "0.0.136", "dependencies": { "tslib": "^2.3.0" }, diff --git a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts index de4cc212..3c107dba 100644 --- a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts +++ b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts @@ -123,7 +123,12 @@ const addDimensionToSQLProjection = ( if (!dimensionSchema) { continue; } - // Gate on emitted count, not loop index, so a skipped unresolved dimension doesn't leave a leading comma. + // `dimensions` is caller-supplied and not guaranteed to only contain names present + // in `tableSchemas` (e.g. a caller can pass a field that's deliberately excluded + // from the schema, such as a hidden field). When that happens `!resolved` or + // `!dimensionSchema` above skips it via `continue`, but `i` still advances — so + // gating on loop index left a leading comma before the next real dimension. Gate + // on whether one was actually emitted instead. if (hasEmittedDimension) { newSelectString += ','; } diff --git a/meerkat-node/package.json b/meerkat-node/package.json index 8e7c8878..8944df68 100644 --- a/meerkat-node/package.json +++ b/meerkat-node/package.json @@ -1,6 +1,6 @@ { "name": "@devrev/meerkat-node", - "version": "0.0.135", + "version": "0.0.136", "dependencies": { "@devrev/meerkat-core": "*", "@swc/helpers": "~0.5.0", From 6c755c0154350348fbed91bb6d42cf32a36624b6 Mon Sep 17 00:00:00 2001 From: Johnson Abraham Date: Tue, 22 Sep 2026 16:50:27 +0530 Subject: [PATCH 3/5] refactor(CEMAS-103): collect entries and join instead of tracking loop state Per review feedback: build an array of resolved entries and join them with the separator, rather than tracking loop index / an emitted-flag to decide when to prepend a comma. Removes the need to reason about what happened in a previous iteration, and applies the same pattern to cubeMeasureToSQLSelectString for consistency. work-item: CEMAS-103 --- .../cube-measure-transformer.ts | 50 ++++++------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts index 3c107dba..0c1d973f 100644 --- a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts +++ b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts @@ -27,14 +27,14 @@ export const cubeMeasureToSQLSelectString = ( measures: Member[], tableSchemas: TableSchema[] ) => { - let base = 'SELECT'; - for (let i = 0; i < measures.length; i++) { - const measure = measures[i]; + const entries: string[] = []; + + for (const measure of measures) { if (measure === '*') { // `*` is a single-table convenience — preserve original behavior by // emitting `.*`. Multi-table joined schemas should not // combine `*` with named measures. - base += ` ${tableSchemas[0].name}.*`; + entries.push(` ${tableSchemas[0].name}.*`); continue; } const resolved = findSchemaForMember(measure, tableSchemas); @@ -56,9 +56,6 @@ export const cubeMeasureToSQLSelectString = ( if (!measureSchema) { continue; } - if (i > 0) { - base += ', '; - } let meerkatReplacedSqlString = meerkatPlaceholderReplacer( measureSchema.sql, @@ -76,8 +73,6 @@ export const cubeMeasureToSQLSelectString = ( meerkatReplacedSqlString, tableSchemaName ); - - //Replace all the columnsUsedInMeasure with safeKey columnsUsedInMeasure?.forEach((measureKey) => { const [, column] = splitIntoDataSourceAndFields(measureKey); const memberKey = getNamespacedKey(tableSchemaName, column); @@ -88,9 +83,10 @@ export const cubeMeasureToSQLSelectString = ( ); }); - base += ` ${meerkatReplacedSqlString} AS ${aliasKey} `; + entries.push(` ${meerkatReplacedSqlString} AS ${aliasKey} `); } - return base; + + return `SELECT${entries.join(', ')}`; }; const addDimensionToSQLProjection = ( @@ -98,13 +94,9 @@ const addDimensionToSQLProjection = ( selectString: string, tableSchemas: TableSchema[] ) => { - if (dimensions.length === 0) { - return selectString; - } - let newSelectString = selectString; - let hasEmittedDimension = false; - for (let i = 0; i < dimensions.length; i++) { - const dimension = dimensions[i]; + const entries: string[] = []; + + for (const dimension of dimensions) { const resolved = findSchemaForMember(dimension, tableSchemas); if (!resolved) { continue; @@ -112,31 +104,17 @@ const addDimensionToSQLProjection = ( const { schema: ownerSchema } = resolved; const [, dimensionKeyWithoutTable] = splitIntoDataSourceAndFields(dimension); - // See comment in `cubeMeasureToSQLSelectString` — resolve the dimension - // in its own source-table schema so duplicate names across joined tables - // don't collide. const dimensionSchema = ownerSchema.dimensions.find( (m) => m.name === dimensionKeyWithoutTable ); - const aliasKey = getAliasForSQL(dimension, ownerSchema); - if (!dimensionSchema) { continue; } - // `dimensions` is caller-supplied and not guaranteed to only contain names present - // in `tableSchemas` (e.g. a caller can pass a field that's deliberately excluded - // from the schema, such as a hidden field). When that happens `!resolved` or - // `!dimensionSchema` above skips it via `continue`, but `i` still advances — so - // gating on loop index left a leading comma before the next real dimension. Gate - // on whether one was actually emitted instead. - if (hasEmittedDimension) { - newSelectString += ','; - } - // since alias key is expected to have been unfurled in the base query, we can just use it as is. - newSelectString += ` ${aliasKey}`; - hasEmittedDimension = true; + const aliasKey = getAliasForSQL(dimension, ownerSchema); + entries.push(` ${aliasKey}`); } - return newSelectString; + + return selectString + entries.join(','); }; export const getSelectReplacedSql = (sql: string, selectString: string) => { From 00567149f27ed8b36170c7f42c175615c12116e6 Mon Sep 17 00:00:00 2001 From: Johnson Abraham Date: Tue, 22 Sep 2026 16:54:27 +0530 Subject: [PATCH 4/5] fix(CEMAS-103): restore pre-existing comments dropped in comment cleanup The prior commit's comment trim removed three comments that predate this change and document unrelated behavior: - the source-table resolution note above dimensionSchema lookup - the safeKey replacement note above the forEach - the alias-key note above the dimension push Restore all three. work-item: CEMAS-103 --- .../cube-measure-transformer/cube-measure-transformer.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts index 0c1d973f..1d7ad92c 100644 --- a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts +++ b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts @@ -73,6 +73,8 @@ export const cubeMeasureToSQLSelectString = ( meerkatReplacedSqlString, tableSchemaName ); + + //Replace all the columnsUsedInMeasure with safeKey columnsUsedInMeasure?.forEach((measureKey) => { const [, column] = splitIntoDataSourceAndFields(measureKey); const memberKey = getNamespacedKey(tableSchemaName, column); @@ -104,13 +106,18 @@ const addDimensionToSQLProjection = ( const { schema: ownerSchema } = resolved; const [, dimensionKeyWithoutTable] = splitIntoDataSourceAndFields(dimension); + // See comment in `cubeMeasureToSQLSelectString` — resolve the dimension + // in its own source-table schema so duplicate names across joined tables + // don't collide. const dimensionSchema = ownerSchema.dimensions.find( (m) => m.name === dimensionKeyWithoutTable ); + const aliasKey = getAliasForSQL(dimension, ownerSchema); + if (!dimensionSchema) { continue; } - const aliasKey = getAliasForSQL(dimension, ownerSchema); + // since alias key is expected to have been unfurled in the base query, we can just use it as is. entries.push(` ${aliasKey}`); } From 2bd6c412cf12bdf01e78d188664eb45b4084c4a4 Mon Sep 17 00:00:00 2001 From: Johnson Abraham Date: Tue, 22 Sep 2026 21:24:46 +0530 Subject: [PATCH 5/5] fix(CEMAS-103): revert unrelated refactor of cubeMeasureToSQLSelectString Per review feedback: cubeMeasureToSQLSelectString was never part of the reported bug and shouldn't be structurally changed in this PR. Restored it to its original loop-based form, byte-identical to main. addDimensionToSQLProjection keeps the collect-and-join fix, which is what the regression test covers. work-item: CEMAS-103 --- .../cube-measure-transformer.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts index 1d7ad92c..f4aa94c5 100644 --- a/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts +++ b/meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts @@ -27,14 +27,14 @@ export const cubeMeasureToSQLSelectString = ( measures: Member[], tableSchemas: TableSchema[] ) => { - const entries: string[] = []; - - for (const measure of measures) { + let base = 'SELECT'; + for (let i = 0; i < measures.length; i++) { + const measure = measures[i]; if (measure === '*') { // `*` is a single-table convenience — preserve original behavior by // emitting `.*`. Multi-table joined schemas should not // combine `*` with named measures. - entries.push(` ${tableSchemas[0].name}.*`); + base += ` ${tableSchemas[0].name}.*`; continue; } const resolved = findSchemaForMember(measure, tableSchemas); @@ -56,6 +56,9 @@ export const cubeMeasureToSQLSelectString = ( if (!measureSchema) { continue; } + if (i > 0) { + base += ', '; + } let meerkatReplacedSqlString = meerkatPlaceholderReplacer( measureSchema.sql, @@ -85,10 +88,9 @@ export const cubeMeasureToSQLSelectString = ( ); }); - entries.push(` ${meerkatReplacedSqlString} AS ${aliasKey} `); + base += ` ${meerkatReplacedSqlString} AS ${aliasKey} `; } - - return `SELECT${entries.join(', ')}`; + return base; }; const addDimensionToSQLProjection = (