Skip to content

Add disableRowMaterialization option to skip per-cell decode - #465

Open
mani-mathur-arch wants to merge 2 commits into
mainfrom
disable-row-materialization
Open

Add disableRowMaterialization option to skip per-cell decode#465
mani-mathur-arch wants to merge 2 commits into
mainfrom
disable-row-materialization

Conversation

@mani-mathur-arch

@mani-mathur-arch mani-mathur-arch commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds an opt-in connection option, disableRowMaterialization (default false), that lets a caller drain a result set for its row count and fetch/parse cost without paying the per-cell row-materialization overhead.

When enabled, ArrowResultConverter still fetches, decompresses and IPC-parses every Arrow batch, but skips table.toArray() + getRows() (the recursive convertArrowTypesconvertThriftTypes pass) and returns null row placeholders instead. Only the row count is meaningful on this path — the returned objects carry no cell values.

Motivation

The Node driver materializes every cell into a JS object on every fetch path, which dominates wall-clock on large result sets and makes throughput benchmarks measure decode cost rather than fetch cost. This flag gives benchmarks (and any count-only consumer) a way to isolate fetch/parse from per-row decode. It does not change any default behavior.

Changes

  • disableRowMaterialization?: boolean added to ConnectionOptions (IDBSQLClient.ts) and ClientConfig (IClientContext.ts), defaulted false in DBSQLClient.getDefaultConfig and copied from connect() options — same plumbing as preserveBigNumericPrecision.
  • ArrowResultConverter.fetchNext returns new Array(rowCount).fill(null) when the flag is set (filled, not sparse, so fetchAll's .flat() preserves the count).
  • Wired through both backends: Thrift (ARROW_BASED_SET + URL_BASED_SET/CloudFetch) and kernel.

What it does not do

The flag skips per-cell materialization, not the Arrow IPC batch parse (RecordBatchReader.from / new Table), which still runs. So it brings fetch to an "Arrow batches parsed, rows not materialized" level, identical on both the Thrift and kernel backends.

How is this tested?

  • New unit test in tests/unit/result/ArrowResultConverter.test.tsreturns null placeholders with exact counts when disableRowMaterialization is set — mirrors the existing multi-batch "should respect row count in batch" case and asserts each fetch returns the exact row count with every element null.
  • Verified locally: full build clean, eslint clean on changed files, tsc --noEmit adds no new errors (the pre-existing examples/tokenFederation module-resolution errors are present on main too), and 155 unit tests pass across the touched suites (ArrowResultConverter, ResultSlicer, DBSQLClient, DBSQLOperation).

New opt-in connection option (default off). When set, the Arrow converter
still fetches, decompresses and parses each batch but returns `null` row
placeholders instead of running the per-cell type conversion, so only the
row count is meaningful. Wired through both the Thrift and kernel backends
via ClientConfig, mirroring preserveBigNumericPrecision. Lets throughput
benchmarks measure fetch cost without the per-row materialization overhead.

Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
@github-actions

Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

…zation

Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No issues identified by the review bot.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No issues identified by the review bot.

@msrathore-db msrathore-db left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add support for kernel mode

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