Skip to content

Commit 9a9226d

Browse files
committed
Better group by support with aliases
1 parent da1e21e commit 9a9226d

File tree

2 files changed

+40
-3
lines changed

2 files changed

+40
-3
lines changed

internal-packages/tsql/src/query/printer.test.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,16 +1334,43 @@ describe("Virtual columns", () => {
13341334
});
13351335

13361336
describe("GROUP BY clause", () => {
1337-
it("should expand virtual column in GROUP BY", () => {
1337+
it("should use alias for virtual column in GROUP BY", () => {
13381338
const ctx = createVirtualColumnContext();
13391339
const { sql } = printQuery(
13401340
"SELECT is_long_running, count(*) FROM runs GROUP BY is_long_running",
13411341
ctx
13421342
);
13431343

1344-
// Both SELECT and GROUP BY should have the expansion
1344+
// SELECT should have the expression with alias
13451345
expect(sql).toContain(
1346-
"GROUP BY (if(completed_at IS NOT NULL AND started_at IS NOT NULL, dateDiff('second', started_at, completed_at) > 60, 0))"
1346+
"(if(completed_at IS NOT NULL AND started_at IS NOT NULL, dateDiff('second', started_at, completed_at) > 60, 0)) AS is_long_running"
1347+
);
1348+
// GROUP BY should use the alias, not the expression (ClickHouse allows this)
1349+
expect(sql).toContain("GROUP BY is_long_running");
1350+
// Should NOT have the expression in GROUP BY
1351+
expect(sql).not.toContain(
1352+
"GROUP BY (if(completed_at IS NOT NULL AND started_at IS NOT NULL"
1353+
);
1354+
});
1355+
1356+
it("should use alias for virtual column in GROUP BY with WHERE and aggregation", () => {
1357+
const ctx = createVirtualColumnContext();
1358+
const { sql } = printQuery(
1359+
"SELECT execution_duration, count() AS duration_count FROM runs WHERE execution_duration IS NOT NULL GROUP BY execution_duration ORDER BY duration_count DESC LIMIT 100",
1360+
ctx
1361+
);
1362+
1363+
// SELECT should have the expression with alias
1364+
expect(sql).toContain(
1365+
"(dateDiff('millisecond', started_at, completed_at)) AS execution_duration"
1366+
);
1367+
// GROUP BY should use the alias, not the expression
1368+
expect(sql).toContain("GROUP BY execution_duration");
1369+
// Should NOT have the expression in GROUP BY
1370+
expect(sql).not.toContain("GROUP BY (dateDiff('millisecond'");
1371+
// WHERE should still use the expression
1372+
expect(sql).toContain(
1373+
"isNotNull((dateDiff('millisecond', started_at, completed_at)))"
13471374
);
13481375
});
13491376
});

internal-packages/tsql/src/query/printer.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,16 @@ export class ClickHousePrinter {
18451845
return tableQualifiedChain.map((part) => this.printIdentifierOrIndex(part)).join(".");
18461846
}
18471847

1848+
// In GROUP BY context, for virtual columns (columns with expressions),
1849+
// use the alias name instead of the expression. ClickHouse allows
1850+
// referencing SELECT aliases in GROUP BY.
1851+
if (this.inGroupByContext) {
1852+
const virtualColumnName = this.getVirtualColumnNameForField(node.chain);
1853+
if (virtualColumnName !== null) {
1854+
return this.printIdentifier(virtualColumnName);
1855+
}
1856+
}
1857+
18481858
const virtualExpression = this.getVirtualColumnExpressionForField(node.chain);
18491859
if (virtualExpression !== null) {
18501860
// Return the expression wrapped in parentheses

0 commit comments

Comments
 (0)