Don't include () when generating implicit column names

This commit is contained in:
Matt Aitken
2025-12-20 18:40:52 +00:00
parent 126cf1d43e
commit 62d4113812
2 changed files with 67 additions and 25 deletions
@@ -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");
});
});
});
+30 -17
View File
@@ -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";
}