From 62d41138125a24acb2e71eeca7cf56049ab1ef52 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Sat, 20 Dec 2025 18:40:52 +0000 Subject: [PATCH] Don't include () when generating implicit column names --- .../tsql/src/query/printer.test.ts | 45 ++++++++++++++---- internal-packages/tsql/src/query/printer.ts | 47 ++++++++++++------- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/internal-packages/tsql/src/query/printer.test.ts b/internal-packages/tsql/src/query/printer.test.ts index 36f62ac4f..5d3b36323 100644 --- a/internal-packages/tsql/src/query/printer.test.ts +++ b/internal-packages/tsql/src/query/printer.test.ts @@ -1522,7 +1522,7 @@ describe("Column metadata", () => { const { columns } = printQuery("SELECT COUNT() FROM runs", ctx); expect(columns).toHaveLength(1); - expect(columns[0].name).toBe("count()"); + expect(columns[0].name).toBe("count"); expect(columns[0].type).toBe("UInt64"); }); @@ -1531,7 +1531,7 @@ describe("Column metadata", () => { const { columns } = printQuery("SELECT COUNT(*) FROM runs", ctx); expect(columns).toHaveLength(1); - expect(columns[0].name).toBe("count(*)"); + expect(columns[0].name).toBe("count"); expect(columns[0].type).toBe("UInt64"); }); @@ -1540,7 +1540,7 @@ describe("Column metadata", () => { const { columns } = printQuery("SELECT COUNT(run_id) FROM runs", ctx); expect(columns).toHaveLength(1); - expect(columns[0].name).toBe("count(run_id)"); + expect(columns[0].name).toBe("count"); expect(columns[0].type).toBe("UInt64"); }); @@ -1549,7 +1549,7 @@ describe("Column metadata", () => { const { columns } = printQuery("SELECT SUM(usage_duration_ms) FROM runs", ctx); expect(columns).toHaveLength(1); - expect(columns[0].name).toBe("sum(usage_duration_ms)"); + expect(columns[0].name).toBe("sum"); expect(columns[0].type).toBe("Int64"); }); @@ -1558,7 +1558,7 @@ describe("Column metadata", () => { const { columns } = printQuery("SELECT AVG(usage_duration_ms) FROM runs", ctx); expect(columns).toHaveLength(1); - expect(columns[0].name).toBe("avg(usage_duration_ms)"); + expect(columns[0].name).toBe("avg"); expect(columns[0].type).toBe("Float64"); }); @@ -1570,7 +1570,7 @@ describe("Column metadata", () => { ); expect(columns).toHaveLength(2); - expect(columns[0].name).toBe("count()"); + expect(columns[0].name).toBe("count"); expect(columns[0].type).toBe("UInt64"); expect(columns[1].name).toBe("status"); expect(columns[1].type).toBe("LowCardinality(String)"); @@ -1584,7 +1584,7 @@ describe("Column metadata", () => { ); expect(columns).toHaveLength(1); - expect(columns[0].name).toBe("plus(usage_duration_ms, 100)"); + expect(columns[0].name).toBe("plus"); }); it("should generate implicit name for constant values", () => { @@ -1605,9 +1605,38 @@ describe("Column metadata", () => { expect(columns).toHaveLength(3); expect(columns[0].name).toBe("total"); - expect(columns[1].name).toBe("avg(usage_duration_ms)"); + expect(columns[1].name).toBe("avg"); expect(columns[2].name).toBe("run_status"); }); + + it("should add AS clause to generated SQL for implicit names", () => { + const ctx = createMetadataTestContext(); + const { sql, columns } = printQuery( + "SELECT COUNT(), status FROM runs GROUP BY status", + ctx + ); + + // The SQL should include an explicit AS clause for the COUNT() + expect(sql).toContain("count() AS count"); + expect(columns[0].name).toBe("count"); + }); + + it("should add AS clause for multiple aggregations", () => { + const ctx = createMetadataTestContext(); + const { sql, columns } = printQuery( + "SELECT COUNT(), SUM(usage_duration_ms), AVG(usage_duration_ms) FROM runs", + ctx + ); + + // All aggregations should have AS clauses + expect(sql).toContain("count() AS count"); + expect(sql).toContain("sum(usage_duration_ms) AS sum"); + expect(sql).toContain("avg(usage_duration_ms) AS avg"); + expect(columns).toHaveLength(3); + expect(columns[0].name).toBe("count"); + expect(columns[1].name).toBe("sum"); + expect(columns[2].name).toBe("avg"); + }); }); }); diff --git a/internal-packages/tsql/src/query/printer.ts b/internal-packages/tsql/src/query/printer.ts index 4fe02d681..da05d48d4 100644 --- a/internal-packages/tsql/src/query/printer.ts +++ b/internal-packages/tsql/src/query/printer.ts @@ -501,8 +501,23 @@ export class ClickHousePrinter { sqlResult = visited; } } + } else if ( + // Handle expressions that need implicit aliases (Call, ArithmeticOperation, Constant) + // These expressions get implicit names and need AS clauses so results match metadata + (col as Alias).expression_type !== "alias" && + ((col as Call).expression_type === "call" || + (col as ArithmeticOperation).expression_type === "arithmetic_operation" || + (col as Constant).expression_type === "constant") + ) { + const visited = this.visit(col); + // Add explicit AS clause with the implicit name so ClickHouse results match our metadata + if (outputName) { + sqlResult = `${visited} AS ${this.printIdentifier(outputName)}`; + } else { + sqlResult = visited; + } } else { - // For non-virtual columns or expressions already wrapped in Alias, visit normally + // For Alias expressions or other types, visit normally sqlResult = this.visit(col); } @@ -602,21 +617,21 @@ export class ClickHousePrinter { /** * Generate an implicit column name for an expression without an explicit alias. - * This matches how ClickHouse auto-names result columns. + * This matches how Postgres auto-names result columns (just the function name). * * Examples: - * - COUNT() -> "count()" - * - COUNT(id) -> "count(id)" - * - SUM(duration_ms) -> "sum(duration_ms)" - * - 1 + 2 -> "plus(1, 2)" + * - COUNT() -> "count" + * - COUNT(id) -> "count" + * - SUM(duration_ms) -> "sum" + * - 1 + 2 -> "plus" */ private generateImplicitName(expr: Expression): string { // Handle Call (function/aggregation) if ((expr as Call).expression_type === "call") { const call = expr as Call; - const args = call.args.map((arg) => this.generateImplicitName(arg)); - // Use lowercase function name to match ClickHouse behavior - return `${call.name.toLowerCase()}(${args.join(", ")})`; + // Use lowercase function name without parentheses, like Postgres does + // This allows users to reference the column in WHERE/HAVING without issues + return call.name.toLowerCase(); } // Handle Field @@ -627,23 +642,21 @@ export class ClickHousePrinter { return parts.length > 0 ? parts[parts.length - 1] : "*"; } - // Handle ArithmeticOperation + // Handle ArithmeticOperation - use operator function name without args if ((expr as ArithmeticOperation).expression_type === "arithmetic_operation") { const arith = expr as ArithmeticOperation; - const left = this.generateImplicitName(arith.left); - const right = this.generateImplicitName(arith.right); switch (arith.op) { case ArithmeticOperationOp.Add: - return `plus(${left}, ${right})`; + return "plus"; case ArithmeticOperationOp.Sub: - return `minus(${left}, ${right})`; + return "minus"; case ArithmeticOperationOp.Mult: - return `multiply(${left}, ${right})`; + return "multiply"; case ArithmeticOperationOp.Div: - return `divide(${left}, ${right})`; + return "divide"; case ArithmeticOperationOp.Mod: - return `modulo(${left}, ${right})`; + return "modulo"; default: return "expression"; }