Skip to content

fix(appkit): return numbers and booleans from analytics queries - #709

Open
fjakobs wants to merge 2 commits into
mainfrom
fjakobs/la-2230-numeric-types
Open

fjakobs wants to merge 2 commits into
mainfrom
fjakobs/la-2230-numeric-types

Conversation

@fjakobs

@fjakobs fjakobs commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Why

  • The type generator types numeric columns as number and BOOLEAN columns as boolean in analytics.d.ts.
  • The /query route returns them as strings. The warehouse's JSON_ARRAY format sends every cell as a string, and the connector passed them through unchanged.
  • So useAnalyticsQuery rows had total_trips: "21932" while the type said number. Apps (and agents writing apps) kept adding their own Number(...) workarounds. The dev-playground did the same in its KPI cards.

Reproduced on e2-dogfood (Serverless Starter Warehouse). This is a query run through the real type generator and the connector's JSON_ARRAY path:

column generated type value before
BIGINT number "42"
DECIMAL(10,2) number "1.50"
INT / SMALLINT / TINYINT number "7" / "3" / "1"
DOUBLE / FLOAT number "2.5" / "1.25"
BOOLEAN boolean "true"

Change

  • The SQL warehouse connector attaches the manifest column types (columnTypes) next to the columnNames it already carries. It does this on JSON rows and on inline Arrow results. Cell values are unchanged at this layer.
  • deliverJsonResult takes a coerceScalars option. With it set, coerceScalarCells converts cells by manifest typeName:
    • BYTE, SHORT, INT, LONG, FLOAT, DOUBLE and DECIMAL become numbers.
    • BOOLEAN becomes a boolean.
    • This happens on both the native JSON_ARRAY path and the ARROW_STREAM fallback, so the two paths still match.
  • Only the /query route sets coerceScalars. The /metric route keeps its documented string | null wire values, so metric-view types and docs don't change.
  • NULL stays null. DATE and TIMESTAMP stay ISO strings. STRUCT, ARRAY and MAP values stay as delivered.
  • DOUBLE/FLOAT NaN, Infinity and -Infinity stay the strings "NaN", "Infinity" and "-Infinity". JSON has no number for them, and converting them would put null on the wire, the same as SQL NULL.
  • Dev-playground: remove toNum and the other string-coercion guards in kpi-cards.tsx.
  • Docs: add a "Result value types" note to type-generation.md.

Trade-offs

  • A BIGINT beyond Number.MAX_SAFE_INTEGER and a DECIMAL beyond about 15 significant digits lose precision as JS numbers. DECIMAL also loses trailing zeros ("1.50" becomes 1.5). The docs say to CAST(... AS STRING) for exact values. The generated type then becomes string, so it stays truthful.
  • This changes runtime values on /query: they become what the generated types already claimed. Code that relied on getting strings at runtime will see numbers.
  • Not in scope: nested types (STRUCT, ARRAY, MAP) still arrive as JSON strings although they are typed as unknown[] or Record<string, unknown>.

Testing

  • Unit tests:
    • connector: columnNames and columnTypes are attached.
    • deliverJsonResult: native path, Reyden fallback using the captured fixture, and no conversion without the option.
    • /query route: converts values.
    • /metric route: keeps strings.
  • pnpm build, pnpm docs:build, pnpm check:fix, pnpm -r typecheck and pnpm test all pass (5934 tests).
  • End to end, dev-playground against e2-dogfood:
    • POST /api/analytics/query/dashboard_kpis now returns {"total_trips":21932,"avg_fare":12.35,…}.
    • Sparkline rows have numeric values and trip_date stays "2015-12-31".
    • POST /api/analytics/metric/revenue still returns {"arr":"1.604847408E9"}.
    • The smart-dashboard KPI cards, sparklines and trends render correctly without toNum.
  • After review: a query with every scalar type, typed NULLs and DOUBLE NaN/±Infinity through /query, on both a standard serverless warehouse and a Reyden (REALTIME) warehouse. Every value matches its generated type except the three non-finite strings, and NULLs are null. Also ran the ARROW_STREAM fallback with live Reyden Arrow bytes (the JSON_ARRAY rejection was simulated, since the warehouse now serves JSON_ARRAY): the same values as native JSON_ARRAY.

This pull request and its description were written by Isaac.

JSON_ARRAY returns every cell as a string, so analytics query rows carried
"42" for BIGINT, "1.50" for DECIMAL and "true" for BOOLEAN while the
generated query types promise number and boolean.

The SQL warehouse connector now attaches the manifest column types next to
the column names it already carries, and the /query route converts numeric
and BOOLEAN cells on both the native JSON_ARRAY path and the ARROW_STREAM
fallback. NULL, date/time and nested values are unchanged, and the /metric
route keeps its documented string | null wire values.

Remove the dev-playground's number-coercion workarounds in the KPI cards.

Co-authored-by: Isaac <no-reply@databricks.com>
Signed-off-by: Fabian Jakobs <fabian.jakobs@databricks.com>
@fjakobs
fjakobs requested a review from a team as a code owner November 6, 2026 13:57
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 1.2 MB (+2.3 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 1.3 MB (+2.6 KB) 460 KB (+934 B)
Type declarations 465 KB 169 KB (+1 B)
Source maps 2.5 MB (+4.4 KB) 860 KB (+1.4 KB)
Other 11 KB 3.7 KB
Total 4.3 MB (+6.9 KB) 1.5 MB (+2.3 KB)
Per-entry composition (own code — deps external (as shipped))
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
. 107 KB (+287 B) 2.5 KB 109 KB (+287 B) external 345 KB (+645 B)
./beta 103 KB (+35 B) 482 B 104 KB (+35 B) external 310 KB (+154 B)
./testing 45 KB 32 KB 77 KB external 223 KB
./tsdown 520 B 0 B 520 B external 813 B
./type-generator 26 KB 0 B 26 KB external 74 KB

Chunks:

Entry Chunk Load Size (gz)
. index.js initial 102 KB
. utils.js initial 4.6 KB
. remote-tunnel-manager.js lazy 2.5 KB
./beta beta.js initial 83 KB
./beta modular.js initial 6.0 KB
./beta stream-manager.js initial 5.9 KB
./beta service-context.js initial 4.2 KB
./beta databricks.js initial 3.3 KB
./beta client.js initial 647 B
./beta index.js initial 20 B
./beta supervisor-api.js lazy 192 B
./beta databricks.js lazy 176 B
./beta index.js lazy 114 B
./testing manifest.js initial 32 KB
./testing index.js initial 10 KB
./testing wide-event-emitter.js initial 2.9 KB
./testing index.js lazy 28 KB
./testing remote-tunnel-manager.js lazy 2.5 KB
./testing utils.js lazy 1.8 KB
./tsdown index.js initial 520 B
./type-generator index.js initial 26 KB

@databricks/appkit-ui

npm tarball (packed): 397 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 429 KB 147 KB
Type declarations 267 KB 98 KB
Source maps 853 KB 286 KB
CSS 16 KB 3.2 KB
Total 1.5 MB 534 KB
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
./js 5.3 KB 49 KB 55 KB 208 KB 14 KB
./js/beta 2.1 KB 0 B 2.1 KB 0 B 5.0 KB
./react 433 KB 49 KB 481 KB 1.3 MB 177 KB
./react/beta 4.9 KB 0 B 4.9 KB 0 B 12 KB

Chunks:

Entry Chunk Load Size (gz)
./js index.js initial 5.2 KB
./js chunk initial 120 B
./js apache-arrow lazy 49 KB
./js/beta beta.js initial 2.1 KB
./react index.js initial 431 KB
./react tslib initial 2.1 KB
./react apache-arrow lazy 49 KB
./react/beta beta.js initial 4.9 KB

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 42247154360 -R databricks/appkit -n appkit-template-0.90.0-pr.fbfc70a-fjakobs-la-2230-numeric-types-709 -D appkit-pr-709 \
  && unzip -o "appkit-pr-709/appkit-template-0.90.0-pr.fbfc70a-fjakobs-la-2230-numeric-types-709.zip" -d "appkit-pr-709" \
  && databricks apps init --template "appkit-pr-709"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

@fjakobs
fjakobs requested a review from pffigueiredo November 6, 2026 15:47
Under JSON_ARRAY the warehouse returns DOUBLE and FLOAT NaN and ±Infinity
as "NaN", "Infinity" and "-Infinity". The /query coercion turned them into
NaN/Infinity numbers, which the SSE JSON then sent as null, the same value
as SQL NULL. Numeric cells that don't parse to a finite number now keep the
warehouse string, since JSON has no number for them.

Co-authored-by: Isaac <no-reply@databricks.com>
Signed-off-by: Fabian Jakobs <fabian.jakobs@databricks.com>
@fjakobs

fjakobs commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

@pffigueiredo thanks for the review. I replied in your thread, which is resolved and so collapsed: #709 (comment)

Short version: I kept plain Number() / === "true" instead of zod. .parse throws on the "NaN" / "Infinity" / "-Infinity" strings the warehouse really returns for DOUBLE/FLOAT, and that would fail the whole query. Your .finite() did point at a real bug, though: those values were reaching the client as null, the same as SQL NULL. 4f6d390 keeps them as the warehouse strings and adds tests and a docs note. Could you take another look?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants