Skip to content

Commit d7c0d01

Browse files
committed
fix(tsql): reach through passthrough wrappers for JSON functions on native JSON columns
A JSON function whose JSON argument is wrapped in a value-preserving passthrough (e.g. JSONExtractArrayRaw(assumeNotNull(output), 'x')) still printed against the native JSON column and failed with "illegal type: JSON". Descend through those wrappers so the text-column swap applies inside them, while leaving value-changing wrappers like toJSONString on the native column.
1 parent 4581323 commit d7c0d01

3 files changed

Lines changed: 72 additions & 2 deletions

File tree

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ const defaultTaskRun = {
7272
started_at: Date.now() - 5000,
7373
completed_at: Date.now(),
7474
tags: ["tag-a", "tag-b"],
75-
output: { data: { count: 42, label: "ok", ratio: 1.5, enabled: true } },
75+
output: { data: { count: 42, label: "ok", ratio: 1.5, enabled: true, items: [1, 2, 3] } },
7676
error: null,
7777
usage_duration_ms: 4500,
7878
cost_in_cents: 1.5,
@@ -595,6 +595,17 @@ describe("TSQL Function Smoke Tests", () => {
595595
],
596596
["JSONExtractRaw(output)", "SELECT JSONExtractRaw(output, 'count') AS r FROM task_runs"],
597597
["JSONExtractKeys(output)", "SELECT JSONExtractKeys(output) AS r FROM task_runs"],
598+
// The JSON field can be wrapped in a passthrough like assumeNotNull; the swap
599+
// still has to reach the native column underneath.
600+
[
601+
"JSONExtractArrayRaw(assumeNotNull(output))",
602+
"SELECT JSONExtractArrayRaw(assumeNotNull(output), 'items') AS r FROM task_runs",
603+
],
604+
// toJSONString(output) is already a String, so it stays on the native column.
605+
[
606+
"JSONExtractString(toJSONString(output))",
607+
"SELECT JSONExtractString(toJSONString(output), 'data', 'label') AS r FROM task_runs",
608+
],
598609
]);
599610
});
600611

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,32 @@ describe("ClickHousePrinter", () => {
880880
expect(sql).toContain("JSONExtractInt(runs.output_text,");
881881
});
882882

883+
it("should reach through assumeNotNull() to swap the JSON field for its text column", () => {
884+
const ctx = createTextColumnContext();
885+
const { sql } = printQuery(
886+
"SELECT JSONExtractArrayRaw(assumeNotNull(output), 'losers') AS r FROM runs",
887+
ctx
888+
);
889+
890+
// The JSON field is wrapped in a value-preserving passthrough, so the swap
891+
// has to descend into it: assumeNotNull(output) -> assumeNotNull(output_text).
892+
expect(sql).toContain("JSONExtractArrayRaw(assumeNotNull(output_text),");
893+
expect(sql).not.toContain("assumeNotNull(output)");
894+
});
895+
896+
it("should NOT rewrite a JSON field consumed by toJSONString inside JSONExtract*", () => {
897+
const ctx = createTextColumnContext();
898+
const { sql } = printQuery(
899+
"SELECT JSONExtractString(toJSONString(output), 'x') AS r FROM runs",
900+
ctx
901+
);
902+
903+
// toJSONString(output) is already a String and changes the value, so it must
904+
// stay on the native column.
905+
expect(sql).toContain("toJSONString(output)");
906+
expect(sql).not.toContain("output_text");
907+
});
908+
883909
it("should use text column for table-qualified JSON columns in SELECT", () => {
884910
const ctx = createTextColumnContext();
885911
const { sql } = printQuery("SELECT runs.output FROM runs", ctx);

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2529,6 +2529,31 @@ export class ClickHousePrinter {
25292529
return this.printIdentifier(textColumn);
25302530
}
25312531

2532+
/**
2533+
* Print a JSON-string function argument, swapping a bare JSON field for its textColumn.
2534+
* Descends through value-preserving passthrough wrappers (e.g. assumeNotNull(output))
2535+
* so the swap still applies inside them. Returns null if no textColumn swap applies.
2536+
*/
2537+
private substituteJsonTextArg(expr: Expression): string | null {
2538+
const direct = this.printTextColumnReference(expr);
2539+
if (direct) return direct;
2540+
2541+
const call = expr as Call;
2542+
if (
2543+
call.expression_type === "call" &&
2544+
ClickHousePrinter.JSON_PASSTHROUGH_WRAPPERS.has(call.name.toLowerCase()) &&
2545+
call.args.length > 0
2546+
) {
2547+
const inner = this.substituteJsonTextArg(call.args[0]);
2548+
if (inner) {
2549+
const clickhouseName = findTSQLFunction(call.name)?.clickhouseName ?? call.name;
2550+
const rest = call.args.slice(1).map((arg) => this.visit(arg));
2551+
return `${clickhouseName}(${[inner, ...rest].join(", ")})`;
2552+
}
2553+
}
2554+
return null;
2555+
}
2556+
25322557
/**
25332558
* Get the dataPrefix for a field chain if the root column has one defined.
25342559
* Returns null if the column doesn't have a dataPrefix or if this isn't a subfield access.
@@ -3002,6 +3027,14 @@ export class ClickHousePrinter {
30023027
"jsonextractkeys",
30033028
]);
30043029

3030+
/**
3031+
* Value-preserving wrappers a user may put between a JSON column and a JSON function,
3032+
* e.g. `JSONExtractString(assumeNotNull(output), 'x')`. The text-column swap descends
3033+
* through these. Functions that transform the value (e.g. toJSONString) are excluded so
3034+
* their argument keeps reading the native column.
3035+
*/
3036+
private static readonly JSON_PASSTHROUGH_WRAPPERS = new Set(["assumenotnull"]);
3037+
30053038
/**
30063039
* Visit function call arguments, handling date functions that require an interval unit
30073040
* keyword as their first argument. For these functions, the first arg is output as a
@@ -3020,7 +3053,7 @@ export class ClickHousePrinter {
30203053
}
30213054

30223055
if (ClickHousePrinter.JSON_TEXT_ARG_FUNCTIONS.has(lowerName) && args.length > 0) {
3023-
const textColumn = this.printTextColumnReference(args[0]);
3056+
const textColumn = this.substituteJsonTextArg(args[0]);
30243057
if (textColumn) {
30253058
return [textColumn, ...args.slice(1).map((arg) => this.visit(arg))];
30263059
}

0 commit comments

Comments
 (0)