Skip to content

Commit 4581323

Browse files
committed
fix(tsql): use text column for JSON functions on native JSON columns
JSONExtract*, JSONHas, JSONLength, and JSONType expect a String containing JSON, but the compiler printed them against native JSON columns (like a run's output), which ClickHouse rejects with "illegal type: JSON". Print the column's String companion (its textColumn) for the first argument of these functions when it is a bare JSON field, mirroring the existing SELECT and WHERE behavior.
1 parent a3dca98 commit 4581323

4 files changed

Lines changed: 137 additions & 23 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Query page: extracting fields from a run's output with JSON functions (such as JSONExtractString or JSONExtractInt) no longer fails with an "illegal type: JSON" error.

internal-packages/clickhouse/src/tsqlFunctions.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ const taskRunsSchema: TableSchema = {
2828
completed_at: { name: "completed_at", ...column("Nullable(DateTime64)") },
2929
is_test: { name: "is_test", ...column("UInt8") },
3030
tags: { name: "tags", ...column("Array(String)") },
31+
output: {
32+
name: "output",
33+
...column("JSON"),
34+
nullValue: "'{}'",
35+
textColumn: "output_text",
36+
dataPrefix: "data",
37+
},
3138
usage_duration_ms: { name: "usage_duration_ms", ...column("UInt32") },
3239
cost_in_cents: { name: "cost_in_cents", ...column("Float64") },
3340
attempt: { name: "attempt", ...column("UInt8") },
@@ -65,7 +72,7 @@ const defaultTaskRun = {
6572
started_at: Date.now() - 5000,
6673
completed_at: Date.now(),
6774
tags: ["tag-a", "tag-b"],
68-
output: null,
75+
output: { data: { count: 42, label: "ok", ratio: 1.5, enabled: true } },
6976
error: null,
7077
usage_duration_ms: 4500,
7178
cost_in_cents: 1.5,
@@ -572,6 +579,22 @@ describe("TSQL Function Smoke Tests", () => {
572579
],
573580
["JSONExtractKeys", `SELECT JSONExtractKeys('{"a": 1, "b": 2}') AS r FROM task_runs`],
574581
["toJSONString", "SELECT toJSONString(map('a', 1)) AS r FROM task_runs"],
582+
// The `output` column is a native JSON type, so JSON functions must read its
583+
// String companion (output_text). Without the printer fix these fail with
584+
// "should be a string containing JSON, illegal type: JSON".
585+
["JSONHas(output)", "SELECT JSONHas(output, 'count') AS r FROM task_runs"],
586+
["JSONLength(output)", "SELECT JSONLength(output) AS r FROM task_runs"],
587+
["JSONType(output)", "SELECT JSONType(output, 'count') AS r FROM task_runs"],
588+
["JSONExtractInt(output)", "SELECT JSONExtractInt(output, 'count') AS r FROM task_runs"],
589+
["JSONExtractUInt(output)", "SELECT JSONExtractUInt(output, 'count') AS r FROM task_runs"],
590+
["JSONExtractFloat(output)", "SELECT JSONExtractFloat(output, 'ratio') AS r FROM task_runs"],
591+
["JSONExtractBool(output)", "SELECT JSONExtractBool(output, 'enabled') AS r FROM task_runs"],
592+
[
593+
"JSONExtractString(output)",
594+
"SELECT JSONExtractString(output, 'label') AS r FROM task_runs",
595+
],
596+
["JSONExtractRaw(output)", "SELECT JSONExtractRaw(output, 'count') AS r FROM task_runs"],
597+
["JSONExtractKeys(output)", "SELECT JSONExtractKeys(output) AS r FROM task_runs"],
575598
]);
576599
});
577600

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,63 @@ describe("ClickHousePrinter", () => {
823823
expect(sql).toContain("output_text AS result");
824824
});
825825

826+
it("should use text column as the first arg of JSONExtract* on a bare JSON column", () => {
827+
const ctx = createTextColumnContext();
828+
const { sql } = printQuery("SELECT JSONExtractInt(output, 'count') AS r FROM runs", ctx);
829+
830+
// JSONExtract* needs a String containing JSON, not the native JSON column.
831+
expect(sql).toContain("JSONExtractInt(output_text,");
832+
expect(sql).not.toMatch(/JSONExtractInt\(output,/);
833+
});
834+
835+
it("should use text column for the whole JSONExtract* family and JSONHas/Length/Type", () => {
836+
const ctx = createTextColumnContext();
837+
const cases: Array<[string, string]> = [
838+
["JSONExtract", "SELECT JSONExtract(output, 'x', 'Int64') AS r FROM runs"],
839+
["JSONExtractUInt", "SELECT JSONExtractUInt(output, 'x') AS r FROM runs"],
840+
["JSONExtractFloat", "SELECT JSONExtractFloat(output, 'x') AS r FROM runs"],
841+
["JSONExtractBool", "SELECT JSONExtractBool(output, 'x') AS r FROM runs"],
842+
["JSONExtractString", "SELECT JSONExtractString(output, 'x') AS r FROM runs"],
843+
["JSONExtractRaw", "SELECT JSONExtractRaw(output, 'x') AS r FROM runs"],
844+
["JSONExtractArrayRaw", "SELECT JSONExtractArrayRaw(output, 'x') AS r FROM runs"],
845+
[
846+
"JSONExtractKeysAndValues",
847+
"SELECT JSONExtractKeysAndValues(output, 'String') AS r FROM runs",
848+
],
849+
["JSONExtractKeys", "SELECT JSONExtractKeys(output) AS r FROM runs"],
850+
["JSONHas", "SELECT JSONHas(output, 'x') AS r FROM runs"],
851+
["JSONLength", "SELECT JSONLength(output) AS r FROM runs"],
852+
["JSONType", "SELECT JSONType(output, 'x') AS r FROM runs"],
853+
];
854+
855+
for (const [fn, query] of cases) {
856+
const { sql } = printQuery(query, ctx);
857+
expect(sql, `${fn} should read from output_text`).toContain(`${fn}(output_text`);
858+
expect(sql, `${fn} should not read from the native JSON column`).not.toMatch(
859+
new RegExp(`${fn}\\(output,`)
860+
);
861+
}
862+
});
863+
864+
it("should NOT rewrite a JSONExtract* first arg that is a String literal", () => {
865+
const ctx = createTextColumnContext();
866+
const { sql } = printQuery(`SELECT JSONExtractInt('{"a": 1}', 'a') AS r FROM runs`, ctx);
867+
868+
// String literals are parameterized, never swapped for the text column.
869+
expect(sql).toMatch(/JSONExtractInt\(\{tsql_val_\d+: String\}/);
870+
expect(sql).not.toContain("output_text");
871+
});
872+
873+
it("should qualify the text column with the table alias inside JSONExtract*", () => {
874+
const ctx = createTextColumnContext();
875+
const { sql } = printQuery(
876+
"SELECT JSONExtractInt(runs.output, 'count') AS r FROM runs",
877+
ctx
878+
);
879+
880+
expect(sql).toContain("JSONExtractInt(runs.output_text,");
881+
});
882+
826883
it("should use text column for table-qualified JSON columns in SELECT", () => {
827884
const ctx = createTextColumnContext();
828885
const { sql } = printQuery("SELECT runs.output FROM runs", ctx);

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

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1979,28 +1979,10 @@ export class ClickHousePrinter {
19791979
CompareOperationOp.NotILike,
19801980
];
19811981
const useTextColumn = textColumnOps.includes(node.op);
1982-
const leftTextColumn = useTextColumn ? this.getTextColumnForExpression(node.left) : null;
1983-
1984-
// Build the left side, qualifying the text column with table alias if present
1985-
let left: string;
1986-
if (leftTextColumn) {
1987-
// Check if the field is qualified with a table alias (e.g., r.output)
1988-
// and prepend that alias to the text column to avoid ambiguity in JOINs
1989-
const fieldNode = node.left as Field;
1990-
if (fieldNode.expression_type === "field" && fieldNode.chain.length >= 2) {
1991-
const firstPart = fieldNode.chain[0];
1992-
if (typeof firstPart === "string" && this.tableContexts.has(firstPart)) {
1993-
// The field is qualified with a table alias, prepend it to the text column
1994-
left = this.printIdentifier(firstPart) + "." + this.printIdentifier(leftTextColumn);
1995-
} else {
1996-
left = this.printIdentifier(leftTextColumn);
1997-
}
1998-
} else {
1999-
left = this.printIdentifier(leftTextColumn);
2000-
}
2001-
} else {
2002-
left = this.visit(node.left);
2003-
}
1982+
const leftTextColumn = useTextColumn ? this.printTextColumnReference(node.left) : null;
1983+
1984+
// Build the left side, using the (alias-qualified) text column when available
1985+
const left = leftTextColumn ?? this.visit(node.left);
20041986
const right = this.visit(transformedRight);
20051987

20061988
switch (node.op) {
@@ -2529,6 +2511,24 @@ export class ClickHousePrinter {
25292511
return this.getTextColumnForField((expr as Field).chain);
25302512
}
25312513

2514+
/**
2515+
* Print a bare JSON field as its String companion (textColumn), keeping the table alias
2516+
* when qualified (r.output -> r.output_text). Returns null if there is no textColumn.
2517+
*/
2518+
private printTextColumnReference(expr: Expression): string | null {
2519+
const textColumn = this.getTextColumnForExpression(expr);
2520+
if (!textColumn) return null;
2521+
2522+
const fieldNode = expr as Field;
2523+
if (fieldNode.expression_type === "field" && fieldNode.chain.length >= 2) {
2524+
const firstPart = fieldNode.chain[0];
2525+
if (typeof firstPart === "string" && this.tableContexts.has(firstPart)) {
2526+
return this.printIdentifier(firstPart) + "." + this.printIdentifier(textColumn);
2527+
}
2528+
}
2529+
return this.printIdentifier(textColumn);
2530+
}
2531+
25322532
/**
25332533
* Get the dataPrefix for a field chain if the root column has one defined.
25342534
* Returns null if the column doesn't have a dataPrefix or if this isn't a subfield access.
@@ -2981,6 +2981,27 @@ export class ClickHousePrinter {
29812981
"date_diff",
29822982
]);
29832983

2984+
/**
2985+
* ClickHouse JSON functions whose first argument must be a String containing JSON text.
2986+
* Passing a native JSON column fails with "should be a string containing JSON, illegal
2987+
* type: JSON", so we print the column's String companion (textColumn) for that argument.
2988+
*/
2989+
private static readonly JSON_TEXT_ARG_FUNCTIONS = new Set([
2990+
"jsonhas",
2991+
"jsonlength",
2992+
"jsontype",
2993+
"jsonextract",
2994+
"jsonextractuint",
2995+
"jsonextractint",
2996+
"jsonextractfloat",
2997+
"jsonextractbool",
2998+
"jsonextractstring",
2999+
"jsonextractraw",
3000+
"jsonextractarrayraw",
3001+
"jsonextractkeysandvalues",
3002+
"jsonextractkeys",
3003+
]);
3004+
29843005
/**
29853006
* Visit function call arguments, handling date functions that require an interval unit
29863007
* keyword as their first argument. For these functions, the first arg is output as a
@@ -2998,6 +3019,13 @@ export class ClickHousePrinter {
29983019
}
29993020
}
30003021

3022+
if (ClickHousePrinter.JSON_TEXT_ARG_FUNCTIONS.has(lowerName) && args.length > 0) {
3023+
const textColumn = this.printTextColumnReference(args[0]);
3024+
if (textColumn) {
3025+
return [textColumn, ...args.slice(1).map((arg) => this.visit(arg))];
3026+
}
3027+
}
3028+
30013029
return args.map((arg) => this.visit(arg));
30023030
}
30033031

0 commit comments

Comments
 (0)