Skip to content

Commit 3c56314

Browse files
committed
Fully qualify columns
1 parent 10eccd5 commit 3c56314

File tree

2 files changed

+118
-1
lines changed

2 files changed

+118
-1
lines changed

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,104 @@ describe("ClickHousePrinter", () => {
852852
expect(sql).toContain("like(output_text,");
853853
});
854854
});
855+
856+
describe("JOINs with textColumn", () => {
857+
// Create a second schema with the same JSON column names to test JOIN ambiguity
858+
const runsSchemaWithTextColumn: TableSchema = {
859+
name: "runs",
860+
clickhouseName: "trigger_dev.task_runs_v2",
861+
columns: {
862+
id: { name: "id", ...column("String") },
863+
output: {
864+
name: "output",
865+
...column("JSON"),
866+
nullValue: "'{}'",
867+
textColumn: "output_text",
868+
},
869+
organization_id: { name: "organization_id", ...column("String") },
870+
project_id: { name: "project_id", ...column("String") },
871+
environment_id: { name: "environment_id", ...column("String") },
872+
},
873+
tenantColumns: {
874+
organizationId: "organization_id",
875+
projectId: "project_id",
876+
environmentId: "environment_id",
877+
},
878+
};
879+
880+
const eventsSchemaWithTextColumn: TableSchema = {
881+
name: "events",
882+
clickhouseName: "trigger_dev.task_events_v2",
883+
columns: {
884+
id: { name: "id", ...column("String") },
885+
run_id: { name: "run_id", ...column("String") },
886+
output: {
887+
name: "output",
888+
...column("JSON"),
889+
nullValue: "'{}'",
890+
textColumn: "output_text",
891+
},
892+
organization_id: { name: "organization_id", ...column("String") },
893+
project_id: { name: "project_id", ...column("String") },
894+
environment_id: { name: "environment_id", ...column("String") },
895+
},
896+
tenantColumns: {
897+
organizationId: "organization_id",
898+
projectId: "project_id",
899+
environmentId: "environment_id",
900+
},
901+
};
902+
903+
function createJoinTextColumnContext() {
904+
const schema = createSchemaRegistry([runsSchemaWithTextColumn, eventsSchemaWithTextColumn]);
905+
return createPrinterContext({
906+
schema,
907+
enforcedWhereClause: {
908+
organization_id: { op: "eq", value: "org_test" },
909+
project_id: { op: "eq", value: "proj_test" },
910+
environment_id: { op: "eq", value: "env_test" },
911+
},
912+
});
913+
}
914+
915+
it("should qualify text column with table alias in JOIN WHERE clause to avoid ambiguity", () => {
916+
const ctx = createJoinTextColumnContext();
917+
const { sql } = printQuery(
918+
`SELECT r.id FROM runs r JOIN events e ON r.id = e.run_id WHERE r.output = '{}'`,
919+
ctx
920+
);
921+
922+
// The text column should be table-qualified to avoid ambiguity
923+
// since both tables have an output_text column
924+
expect(sql).toContain("equals(r.output_text,");
925+
// Should NOT have unqualified output_text in the comparison
926+
expect(sql).not.toMatch(/equals\(output_text,/);
927+
});
928+
929+
it("should qualify text column with table alias for LIKE in JOIN", () => {
930+
const ctx = createJoinTextColumnContext();
931+
const { sql } = printQuery(
932+
`SELECT r.id FROM runs r JOIN events e ON r.id = e.run_id WHERE e.output LIKE '%error%'`,
933+
ctx
934+
);
935+
936+
// Should use table-qualified text column
937+
expect(sql).toContain("like(e.output_text,");
938+
expect(sql).not.toMatch(/like\(output_text,/);
939+
});
940+
941+
it("should handle multiple qualified text column comparisons in JOIN", () => {
942+
const ctx = createJoinTextColumnContext();
943+
const { sql } = printQuery(
944+
`SELECT r.id FROM runs r JOIN events e ON r.id = e.run_id WHERE r.output = '{}' AND e.output != '{}'`,
945+
ctx
946+
);
947+
948+
// Both comparisons should be table-qualified
949+
expect(sql).toContain("equals(r.output_text,");
950+
expect(sql).toContain("notEquals(e.output_text,");
951+
});
952+
});
855953
});
856954

857955
describe("dataPrefix for JSON columns", () => {

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1890,7 +1890,26 @@ export class ClickHousePrinter {
18901890
const useTextColumn = textColumnOps.includes(node.op);
18911891
const leftTextColumn = useTextColumn ? this.getTextColumnForExpression(node.left) : null;
18921892

1893-
const left = leftTextColumn ? this.printIdentifier(leftTextColumn) : this.visit(node.left);
1893+
// Build the left side, qualifying the text column with table alias if present
1894+
let left: string;
1895+
if (leftTextColumn) {
1896+
// Check if the field is qualified with a table alias (e.g., r.output)
1897+
// and prepend that alias to the text column to avoid ambiguity in JOINs
1898+
const fieldNode = node.left as Field;
1899+
if (fieldNode.expression_type === "field" && fieldNode.chain.length >= 2) {
1900+
const firstPart = fieldNode.chain[0];
1901+
if (typeof firstPart === "string" && this.tableContexts.has(firstPart)) {
1902+
// The field is qualified with a table alias, prepend it to the text column
1903+
left = this.printIdentifier(firstPart) + "." + this.printIdentifier(leftTextColumn);
1904+
} else {
1905+
left = this.printIdentifier(leftTextColumn);
1906+
}
1907+
} else {
1908+
left = this.printIdentifier(leftTextColumn);
1909+
}
1910+
} else {
1911+
left = this.visit(node.left);
1912+
}
18941913
const right = this.visit(transformedRight);
18951914

18961915
switch (node.op) {

0 commit comments

Comments
 (0)