Skip to content

Commit f3cba91

Browse files
committed
Fix for WHERE clause with JSON path
1 parent 42ad273 commit f3cba91

File tree

2 files changed

+76
-3
lines changed

2 files changed

+76
-3
lines changed

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

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,61 @@ describe("ClickHousePrinter", () => {
597597
expect(sql).toContain("GROUP BY status");
598598
expect(sql).not.toContain(".:String");
599599
});
600+
601+
it("should NOT add .:String type hint for JSON subfield in WHERE comparison", () => {
602+
const ctx = createJsonContext();
603+
const { sql } = printQuery(
604+
"SELECT id FROM runs WHERE error.data.name = 'test'",
605+
ctx
606+
);
607+
608+
// WHERE clause should NOT have .:String type hint (it breaks the query)
609+
expect(sql).toContain("equals(error.data.name,");
610+
expect(sql).not.toContain("error.data.name.:String");
611+
});
612+
613+
it("should NOT add .:String for JSON subfield in WHERE with LIKE", () => {
614+
const ctx = createJsonContext();
615+
const { sql } = printQuery(
616+
"SELECT id FROM runs WHERE error.message LIKE '%error%'",
617+
ctx
618+
);
619+
620+
// WHERE clause should NOT have .:String type hint
621+
expect(sql).toContain("like(error.message,");
622+
expect(sql).not.toContain("error.message.:String");
623+
});
624+
625+
it("should add .:String in SELECT but not in WHERE for same field", () => {
626+
const ctx = createJsonContext();
627+
const { sql } = printQuery(
628+
"SELECT error.data.name FROM runs WHERE error.data.name = 'test'",
629+
ctx
630+
);
631+
632+
// SELECT should have .:String with alias
633+
expect(sql).toContain("error.data.name.:String AS error_data_name");
634+
// WHERE should NOT have .:String
635+
expect(sql).toContain("equals(error.data.name,");
636+
// Make sure WHERE doesn't accidentally get the type hint
637+
expect(sql).not.toMatch(/equals\(error\.data\.name\.:String/);
638+
});
639+
640+
it("should add .:String in GROUP BY but not in WHERE for same query", () => {
641+
const ctx = createJsonContext();
642+
const { sql } = printQuery(
643+
"SELECT error.data.name, count() AS cnt FROM runs WHERE error.data.name = 'test' GROUP BY error.data.name",
644+
ctx
645+
);
646+
647+
// SELECT should have .:String
648+
expect(sql).toContain("error.data.name.:String AS error_data_name");
649+
// GROUP BY should have .:String
650+
expect(sql).toContain("GROUP BY error.data.name.:String");
651+
// WHERE should NOT have .:String
652+
expect(sql).toContain("equals(error.data.name,");
653+
expect(sql).not.toMatch(/equals\(error\.data\.name\.:String/);
654+
});
600655
});
601656

602657
describe("textColumn optimization for JSON columns", () => {
@@ -716,16 +771,18 @@ describe("ClickHousePrinter", () => {
716771
expect(sql).toContain("notLike(output_text,");
717772
});
718773

719-
it("should use JSON column for subfield comparison", () => {
774+
it("should use JSON column for subfield comparison without .:String", () => {
720775
const ctx = createTextColumnContext();
721776
const { sql } = printQuery(
722777
"SELECT id FROM runs WHERE output.data.name = 'test'",
723778
ctx
724779
);
725780

726781
// Should use the original JSON column, not the text column
727-
expect(sql).toContain("equals(output.data.name.:String,");
782+
// And should NOT have .:String in WHERE (breaks the query)
783+
expect(sql).toContain("equals(output.data.name,");
728784
expect(sql).not.toContain("output_text");
785+
expect(sql).not.toContain("output.data.name.:String");
729786
});
730787

731788
it("should still use nullValue transformation for IS NULL", () => {

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2126,10 +2126,12 @@ export class ClickHousePrinter {
21262126
// For JSON column subfield access (e.g., error.data.name), add .:String type hint
21272127
// This is required because ClickHouse's Dynamic/Variant types are not allowed in
21282128
// GROUP BY without type casting, and SELECT/GROUP BY expressions must match
2129+
// However, we skip this in WHERE comparisons where it breaks the query
21292130
if (resolvedChain.length > 1) {
21302131
// Check if the root column (first part) is a JSON column
21312132
const rootColumnSchema = this.resolveFieldToColumnSchema([node.chain[0]]);
2132-
if (rootColumnSchema?.type === "JSON") {
2133+
// Add .:String for SELECT/GROUP BY, but NOT in WHERE comparisons
2134+
if (rootColumnSchema?.type === "JSON" && !this.isInWhereComparisonContext()) {
21332135
// Add .:String type hint for JSON subfield access
21342136
result = `${result}.:String`;
21352137
}
@@ -2157,6 +2159,20 @@ export class ClickHousePrinter {
21572159
return false;
21582160
}
21592161

2162+
/**
2163+
* Check if we're inside a WHERE/HAVING comparison operation.
2164+
* Unlike isInComparisonContext(), this does NOT include GROUP BY context.
2165+
* Used to skip .:String type hints in WHERE clauses where they break queries.
2166+
*/
2167+
private isInWhereComparisonContext(): boolean {
2168+
for (const node of this.stack) {
2169+
if ((node as CompareOperation).expression_type === "compare_operation") {
2170+
return true;
2171+
}
2172+
}
2173+
return false;
2174+
}
2175+
21602176
/**
21612177
* Resolve field chain with table alias prefix to avoid alias conflicts.
21622178
* This is used in WHERE clauses when a column has whereTransform to ensure

0 commit comments

Comments
 (0)