Skip to content

Commit c3b20ef

Browse files
committed
fix(appkit): decode ARROW_STREAM DESCRIBE results in typegen
SDK executeStatement returns ARROW_STREAM by default (rows in result.attachment, data_array empty), so the metric and query DESCRIBE fetchers silently degraded on warehouses that don't default to JSON_ARRAY. Add normalizeResultRows (apache-arrow tableFromIPC) and request ARROW_STREAM + INLINE in both fetchers; downstream parsers read the populated data_array unchanged. Verified live against a real warehouse: real measure/dimension unions, cache no longer degraded. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
1 parent ab9a942 commit c3b20ef

10 files changed

Lines changed: 452 additions & 4 deletions

File tree

packages/appkit/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
"@opentelemetry/sdk-trace-base": "2.6.0",
7878
"@opentelemetry/semantic-conventions": "1.38.0",
7979
"@types/semver": "7.7.1",
80+
"apache-arrow": "21.1.0",
8081
"dotenv": "16.6.1",
8182
"express": "4.22.0",
8283
"get-port": "7.2.0",

packages/appkit/src/type-generator/metric-registry.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import fs from "node:fs/promises";
22
import path from "node:path";
33
import type { WorkspaceClient } from "@databricks/sdk-experimental";
4+
import { normalizeResultRows } from "./statement-result";
45
import type { DatabricksStatementExecutionResponse } from "./types";
56

67
/**
@@ -1136,8 +1137,16 @@ export function createWorkspaceDescribeFetcher(
11361137
statement: `DESCRIBE TABLE EXTENDED ${quotedFqn} AS JSON`,
11371138
warehouse_id: warehouseId,
11381139
wait_timeout: "30s",
1140+
// INLINE + ARROW_STREAM returns the single DESCRIBE row as a base64
1141+
// Arrow IPC attachment (the SDK's default disposition would also stream
1142+
// it as an attachment, but pinning these makes the wire shape explicit).
1143+
// normalizeResultRows decodes that attachment into `result.data_array`
1144+
// so `parseDescribeTableExtendedJson` can read the JSON-string cell;
1145+
// it is a no-op passthrough when the warehouse already populated rows.
1146+
format: "ARROW_STREAM",
1147+
disposition: "INLINE",
11391148
})) as DatabricksStatementExecutionResponse;
1140-
return result;
1149+
return await normalizeResultRows(result);
11411150
};
11421151
}
11431152

packages/appkit/src/type-generator/query-registry.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createLogger } from "../logging/logger";
66
import { CACHE_VERSION, hashSQL, loadCache, saveCache } from "./cache";
77
import { decidePreflight, type PreflightMode } from "./preflight";
88
import { Spinner } from "./spinner";
9+
import { normalizeResultRows } from "./statement-result";
910
import {
1011
type DatabricksStatementExecutionResponse,
1112
type QueryFatalError,
@@ -684,6 +685,16 @@ export async function generateQueriesFromDescribe(
684685
const result = (await client.statementExecution.executeStatement({
685686
statement: `DESCRIBE QUERY ${cleanedSql}`,
686687
warehouse_id: warehouseId,
688+
// Wait synchronously for completion (matches the metric fetcher):
689+
// without it the call can return PENDING/RUNNING with no rows yet,
690+
// which classifies as a no-result degrade and ships `unknown`.
691+
wait_timeout: "30s",
692+
// INLINE + ARROW_STREAM returns the DESCRIBE rows as a base64 Arrow
693+
// IPC attachment; normalizeResultRows (below) decodes it into
694+
// `result.data_array` so convertToQueryType can read the columns.
695+
// No-op passthrough when the warehouse already populated rows.
696+
format: "ARROW_STREAM",
697+
disposition: "INLINE",
687698
})) as DatabricksStatementExecutionResponse;
688699

689700
completed++;
@@ -732,7 +743,16 @@ export async function generateQueriesFromDescribe(
732743
};
733744
}
734745

735-
const { type, hasResults } = convertToQueryType(result, sql, queryName);
746+
// Decode an Arrow IPC attachment into `result.data_array` (no-op when
747+
// the warehouse already populated rows) so convertToQueryType can read
748+
// the described columns. State classification above intentionally stays
749+
// on the raw `result` — the normalizer preserves `status` untouched.
750+
const normalized = await normalizeResultRows(result);
751+
const { type, hasResults } = convertToQueryType(
752+
normalized,
753+
sql,
754+
queryName,
755+
);
736756
if (!hasResults) {
737757
// Described, but no result columns. Emit `unknown` and retry next run;
738758
// do not cache (we never persist `result: unknown`).
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import type { DatabricksStatementExecutionResponse } from "./types";
2+
3+
/**
4+
* Normalize a Statement Execution response so downstream parsers can always
5+
* read rows from `result.data_array`, regardless of the wire format the
6+
* warehouse chose.
7+
*
8+
* ## Why this exists
9+
*
10+
* `@databricks/sdk-experimental`'s `executeStatement` defaults to an
11+
* `ARROW_STREAM` disposition. With an `INLINE` disposition the single
12+
* DESCRIBE row is returned as a base64-encoded Arrow IPC stream in
13+
* `result.attachment` and `result.data_array` is left undefined. The metric
14+
* and query type generators only ever read `result.data_array`, so without
15+
* this normalization an Arrow response reads as "returned no rows" — the
16+
* registry ships empty and the runtime fail-closed gate 503s every affected
17+
* metric/query. (A warehouse configured to return `JSON_ARRAY` populates
18+
* `data_array` directly and needs no decoding — that path, and every mocked
19+
* test, flows through here unchanged.)
20+
*
21+
* ## Behavior
22+
*
23+
* - `data_array` already present → return the response unchanged (passthrough;
24+
* keeps JSON_ARRAY warehouses and all `data_array`-based mocked tests working
25+
* with zero decode cost).
26+
* - otherwise `attachment` present → lazily import `apache-arrow`, decode the
27+
* IPC stream, and return a response with `result.data_array` populated as
28+
* `(string | null)[][]`. All other fields (`status`, `statement_id`,
29+
* `manifest`, and any other `result` keys) are preserved.
30+
* - neither present → return the response unchanged (empty result; the
31+
* downstream "returned no rows" path then degrades correctly).
32+
*
33+
* ## Failure contract
34+
*
35+
* Decode is best-effort and never throws: a malformed/empty attachment
36+
* resolves to the original response (with `data_array` still absent) rather
37+
* than rejecting. This is deliberate — the metric/query sync paths treat a
38+
* response without rows as a deterministic "no rows" degrade (warn-and-continue
39+
* + sticky cache), so swallowing the decode error here keeps that contract
40+
* intact instead of crashing the whole generation pass on one bad payload.
41+
*
42+
* The `apache-arrow` import is lazy (dynamic `import()`) so the dependency only
43+
* loads when an attachment actually needs decoding — JSON_ARRAY warehouses and
44+
* unit tests that build `data_array` directly never pull it in.
45+
*
46+
* @param response - the raw Statement Execution response
47+
* @returns a response guaranteed to expose rows via `result.data_array` when
48+
* they were decodable, otherwise the response unchanged
49+
*/
50+
export async function normalizeResultRows(
51+
response: DatabricksStatementExecutionResponse,
52+
): Promise<DatabricksStatementExecutionResponse> {
53+
// Passthrough: rows already materialized (JSON_ARRAY warehouses + every
54+
// mocked test). `data_array` being an empty array still counts as present —
55+
// that is a genuine "no rows" answer we must not overwrite with a decode.
56+
if (response.result?.data_array !== undefined) {
57+
return response;
58+
}
59+
60+
const attachment = response.result?.attachment;
61+
if (attachment === undefined) {
62+
// No rows, no attachment: nothing to normalize. Let the downstream
63+
// "returned no rows" degrade path fire.
64+
return response;
65+
}
66+
67+
try {
68+
// Lazy import: only pull apache-arrow into the process when an attachment
69+
// genuinely needs decoding.
70+
const { tableFromIPC } = await import("apache-arrow");
71+
const bytes = Buffer.from(attachment, "base64");
72+
const table = tableFromIPC(bytes);
73+
const dataArray: (string | null)[][] = table
74+
.toArray()
75+
.map((row) =>
76+
Object.values(row as Record<string, unknown>).map((value) =>
77+
value == null ? null : String(value),
78+
),
79+
);
80+
81+
return {
82+
...response,
83+
result: {
84+
...response.result,
85+
data_array: dataArray,
86+
},
87+
};
88+
} catch {
89+
// Best-effort: a corrupt/partial Arrow payload must not crash the
90+
// generation pass. Returning the response unchanged (data_array still
91+
// absent) routes it into the deterministic "no rows" degrade downstream.
92+
return response;
93+
}
94+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/////3gAAAAQAAAAAAAKAAwACgAJAAQACgAAABAAAAAAAQQACAAIAAAABAAIAAAABAAAAAEAAAAUAAAAEAAUABAAAAAPAAQAAAAIABAAAAAYAAAADAAAAAAAAAUQAAAAAAAAAAQABAAEAAAADQAAAGpzb25fbWV0YWRhdGEAAAD/////uAAAABAAAAAMABoAGAAXAAQACAAMAAAAIAAAAEAFAAAAAAAAAAAAAAAAAAMEAAoAGAAMAAgABAAKAAAALAAAABAAAAABAAAAAAAAAAAAAAABAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAADAAAAAAAAAAAAAAABAAAAAAAAAEAAAAAAAAAACAAAAAAAAACAAAAAAAAAAJEEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAJEEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHsiY2F0YWxvZ19uYW1lIjoiYXBwa2l0X2RlbW8iLCJzY2hlbWFfbmFtZSI6InB1YmxpYyIsIm5hbWVzcGFjZSI6WyJwdWJsaWMiXSwidGFibGVfbmFtZSI6InJldmVudWVfbWV0cmljcyIsImNvbHVtbnMiOlt7Im5hbWUiOiJyZWdpb24iLCJudWxsYWJsZSI6dHJ1ZSwidHlwZSI6eyJjb2xsYXRpb24iOiJVVEY4X0JJTkFSWSIsIm5hbWUiOiJzdHJpbmcifX0seyJuYW1lIjoic2VnbWVudCIsIm51bGxhYmxlIjp0cnVlLCJ0eXBlIjp7ImNvbGxhdGlvbiI6IlVURjhfQklOQVJZIiwibmFtZSI6InN0cmluZyJ9fSx7Im5hbWUiOiJjcmVhdGVkX2F0IiwibnVsbGFibGUiOmZhbHNlLCJ0eXBlIjp7Im5hbWUiOiJ0aW1lc3RhbXBfbHR6In19LHsiaXNfbWVhc3VyZSI6dHJ1ZSwibmFtZSI6Im1yciIsIm51bGxhYmxlIjp0cnVlLCJ0eXBlIjp7Im5hbWUiOiJkb3VibGUifX0seyJjb21tZW50IjoiQW5udWFsaXplZCBjb250cmFjdCB2YWx1ZSBhY3Jvc3MgYWxsIGFjdGl2ZSBzdWJzY3JpcHRpb25zIiwiaXNfbWVhc3VyZSI6dHJ1ZSwibmFtZSI6ImFyciIsIm51bGxhYmxlIjp0cnVlLCJ0eXBlIjp7Im5hbWUiOiJkb3VibGUifX0seyJpc19tZWFzdXJlIjp0cnVlLCJuYW1lIjoibmV3X2FyciIsIm51bGxhYmxlIjp0cnVlLCJ0eXBlIjp7Im5hbWUiOiJkb3VibGUifX0seyJpc19tZWFzdXJlIjp0cnVlLCJuYW1lIjoiY2h1cm5lZF9hcnIiLCJudWxsYWJsZSI6dHJ1ZSwidHlwZSI6eyJuYW1lIjoiZG91YmxlIn19XSwidHlwZSI6Ik1FVFJJQ19WSUVXIiwicHJvdmlkZXIiOiJ1bmtub3duIiwibG9jYXRpb24iOiIvIiwiaXNfbWFuYWdlZF9sb2NhdGlvbiI6ZmFsc2UsInRhYmxlX3Byb3BlcnRpZXMiOnsibWV0cmljX3ZpZXcuZnJvbS5uYW1lIjoiYXBwa2l0X2RlbW8ucHVibGljLmZhY3Rfc3Vic2NyaXB0aW9uIiwibWV0cmljX3ZpZXcuZnJvbS50eXBlIjoiQVNTRVQiLCJtZXRyaWNfdmlldy5qb2lucyI6Ilt7XCJuYW1lXCI6XCJjdXN0b21lclwifV0iLCJtZXRyaWNfdmlldy5tYXRlcmlhbGl6YXRpb24uZW5hYmxlZCI6ImZhbHNlIiwibWV0cmljX3ZpZXcud2hlcmUiOiJzb3VyY2Uuc3RhdHVzID0gJ2FjdGl2ZScifSwiY29sbGF0aW9uIjoiVVRGOF9CSU5BUlkiLCJjcmVhdGVkX3RpbWUiOiIyMDI2LTA0LTMwVDA3OjE3OjMyWiIsImxhc3RfYWNjZXNzIjoiVU5LTk9XTiJ9AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/////AAAAAA==

packages/appkit/src/type-generator/tests/generate-queries.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,35 @@ function succeededResult(columns: [string, string, string | null][]) {
8383
};
8484
}
8585

86+
/**
87+
* Build a SUCCEEDED DESCRIBE QUERY response whose rows arrive only as a base64
88+
* Arrow IPC `attachment` (no `data_array`) — the ARROW_STREAM/INLINE wire shape
89+
* the fetcher now requests. The describeOne path pipes this through
90+
* normalizeResultRows, which decodes the attachment so convertToQueryType can
91+
* read the columns. Each [name, type, comment] triple becomes one DESCRIBE row.
92+
*/
93+
async function succeededArrowAttachmentResult(
94+
columns: [string, string, string | null][],
95+
) {
96+
const arrow = await import("apache-arrow");
97+
const table = arrow.tableFromArrays({
98+
col_name: columns.map((c) => c[0]),
99+
data_type: columns.map((c) => c[1]),
100+
comment: columns.map((c) => c[2]),
101+
});
102+
const attachment = Buffer.from(arrow.tableToIPC(table, "stream")).toString(
103+
"base64",
104+
);
105+
return {
106+
statement_id: "stmt-arrow",
107+
status: { state: "SUCCEEDED" },
108+
manifest: { format: "ARROW_STREAM" },
109+
// No data_array — rows live in the attachment, like a real INLINE Arrow
110+
// response. This is the condition the silent-degrade bug left unread.
111+
result: { attachment },
112+
};
113+
}
114+
86115
describe("generateQueriesFromDescribe", () => {
87116
beforeEach(() => {
88117
vi.clearAllMocks();
@@ -120,6 +149,60 @@ describe("generateQueriesFromDescribe", () => {
120149
expect(lastSavedQueries()?.users.type).toContain("id: number");
121150
});
122151

152+
test("ARROW attachment path — decodes Arrow rows into a real query schema", async () => {
153+
// The warehouse answers ARROW_STREAM/INLINE: columns arrive only as a
154+
// base64 Arrow IPC attachment with data_array undefined. describeOne pipes
155+
// this through normalizeResultRows before convertToQueryType, so the schema
156+
// resolves to real columns instead of the degraded `result: unknown`.
157+
mocks.readdir.mockResolvedValue(["users.sql"]);
158+
mocks.readFile.mockResolvedValue(
159+
"SELECT id, name FROM users WHERE status = :status",
160+
);
161+
mocks.executeStatement.mockResolvedValue(
162+
await succeededArrowAttachmentResult([
163+
["id", "INT", null],
164+
["name", "STRING", "display name"],
165+
]),
166+
);
167+
168+
const { schemas, syntaxErrors, fatalErrors } = await describeQueries(
169+
"/queries",
170+
"wh-123",
171+
);
172+
173+
expect(schemas).toHaveLength(1);
174+
expect(schemas[0].name).toBe("users");
175+
// Real columns recovered from the Arrow attachment — not `result: unknown`.
176+
expect(schemas[0].type).toContain("id: number");
177+
expect(schemas[0].type).toContain("name: string");
178+
expect(schemas[0].type).not.toContain("result: unknown");
179+
expect(syntaxErrors).toEqual([]);
180+
expect(fatalErrors).toEqual([]);
181+
// A resolved schema is cached (we only persist non-unknown results).
182+
expect(lastSavedQueries()?.users.type).toContain("id: number");
183+
});
184+
185+
test("ARROW attachment request — pins ARROW_STREAM/INLINE and a 30s wait", async () => {
186+
// Regression guard for the executeStatement call shape: without the pinned
187+
// wait_timeout the call could return PENDING and degrade; without
188+
// ARROW_STREAM/INLINE the normalizer would have no attachment to decode.
189+
mocks.readdir.mockResolvedValue(["q.sql"]);
190+
mocks.readFile.mockResolvedValue("SELECT 1 AS one");
191+
mocks.executeStatement.mockResolvedValue(
192+
succeededResult([["one", "INT", null]]),
193+
);
194+
195+
await describeQueries("/queries", "wh-123");
196+
197+
expect(mocks.executeStatement).toHaveBeenCalledTimes(1);
198+
expect(mocks.executeStatement.mock.calls[0][0]).toMatchObject({
199+
warehouse_id: "wh-123",
200+
wait_timeout: "30s",
201+
format: "ARROW_STREAM",
202+
disposition: "INLINE",
203+
});
204+
});
205+
123206
test("FAILED status with error message — reports SQL error and produces unknown result type", async () => {
124207
mocks.readdir.mockResolvedValue(["bad_table.sql"]);
125208
mocks.readFile.mockResolvedValue("SELECT * FROM bad_table");

packages/appkit/src/type-generator/tests/metric-registry.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { readFileSync } from "node:fs";
12
import fs from "node:fs/promises";
23
import os from "node:os";
34
import path from "node:path";
@@ -46,6 +47,20 @@ function mockDescribeResponse(
4647
};
4748
}
4849

50+
/**
51+
* Real Arrow IPC attachment captured live from dogfood:
52+
* DESCRIBE TABLE EXTENDED `appkit_demo`.`public`.`revenue_metrics` AS JSON
53+
* with `format: "ARROW_STREAM", disposition: "INLINE"`. The single DESCRIBE
54+
* row (one JSON-string cell) is base64 Arrow IPC — the wire shape this fetcher
55+
* now requests and the normalizer decodes. Used to prove the fetcher →
56+
* normalizer → parser chain extracts real columns from an attachment-only
57+
* response (the silent-degrade bug left this unread).
58+
*/
59+
const ARROW_ATTACHMENT_B64 = readFileSync(
60+
path.join(__dirname, "fixtures", "describe-arrow-attachment.b64"),
61+
"utf-8",
62+
);
63+
4964
/**
5065
* Cast helper for fixtures that intentionally violate the config type
5166
* (invalid executors, unknown fields, legacy shapes, ...).
@@ -439,9 +454,48 @@ describe("createWorkspaceDescribeFetcher", () => {
439454
statement: "DESCRIBE TABLE EXTENDED `demo`.`sales`.`revenue` AS JSON",
440455
warehouse_id: "wh-1",
441456
wait_timeout: "30s",
457+
// Pinned wire format: the warehouse returns the single DESCRIBE row as a
458+
// base64 Arrow IPC attachment, which the normalizer decodes into rows.
459+
format: "ARROW_STREAM",
460+
disposition: "INLINE",
442461
});
443462
});
444463

464+
test("decodes an Arrow attachment-only response into parseable columns (fetcher → normalizer → parser)", async () => {
465+
// The warehouse answers ARROW_STREAM/INLINE: rows arrive as a base64 Arrow
466+
// IPC attachment with `data_array` undefined. Before the normalizer was
467+
// wired in, parseDescribeTableExtendedJson read this as "no rows" and the
468+
// metric shipped degraded. Now the fetcher pipes the response through
469+
// normalizeResultRows, so the real describe doc is recovered end-to-end.
470+
const statements: Array<Record<string, unknown>> = [];
471+
const client = {
472+
statementExecution: {
473+
executeStatement: async (req: Record<string, unknown>) => {
474+
statements.push(req);
475+
return {
476+
statement_id: "stmt-arrow",
477+
status: { state: "SUCCEEDED" },
478+
manifest: { format: "ARROW_STREAM" },
479+
// Only an attachment — no data_array (the bug's trigger condition).
480+
result: { attachment: ARROW_ATTACHMENT_B64 },
481+
} as DatabricksStatementExecutionResponse;
482+
},
483+
},
484+
} as unknown as Parameters<typeof createWorkspaceDescribeFetcher>[0];
485+
486+
const fetcher = createWorkspaceDescribeFetcher(client, "wh-1");
487+
const response = await fetcher("appkit_demo.public.revenue_metrics");
488+
489+
// The fetcher decoded the attachment: rows are now readable.
490+
expect(response.result?.data_array).toBeDefined();
491+
const parsed = parseDescribeTableExtendedJson(response);
492+
const cols = extractMetricColumns(parsed);
493+
// The real revenue_metrics describe doc carries measures and dimensions.
494+
expect(cols.length).toBeGreaterThan(0);
495+
expect(cols.some((c) => c.isMeasure)).toBe(true);
496+
expect(cols.some((c) => !c.isMeasure)).toBe(true);
497+
});
498+
445499
test("a hyphenated FQN round-trips: validated by resolveMetricConfig, quoted in the statement, response parsed", async () => {
446500
// Hyphenated catalogs are valid per the shared source regex; unquoted
447501
// they would be a SQL syntax error against a real warehouse.

0 commit comments

Comments
 (0)