Skip to content

test(c/validation): assert a zero-row bound batch still yields a result schema - #24

Open
fornwall wants to merge 4 commits into
mainfrom
validation/bind-zero-rows
Open

test(c/validation): assert a zero-row bound batch still yields a result schema#24
fornwall wants to merge 4 commits into
mainfrom
validation/bind-zero-rows

Conversation

@fornwall

@fornwall fornwall commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Split out from #21 (3/5).

Adds StatementTest.SqlBindZeroRows to the generic C++ validation suite: executing a prepared query with a bound parameter batch of zero rows (e.g. a DBAPI executemany with an empty parameter list) must yield a result stream with zero rows and exactly the same schema as a non-empty execution of the same query — not execute the query with stale or missing parameters, and not report a placeholder schema. The schema is compared deeply against a reference execution with one bound row: format, name, flags, n_children, children (recursively) and dictionary. Also carries the SQLite fix the test exposed: fix(c/driver/sqlite): report the real result schema for a zero-row bound parameter stream — the reader skipped schema inference entirely when the bound parameter stream had zero rows.

  • adbc-spanner finding: COR-9 (zero-row bound batch advertises an empty schema)
  • Spec reference: no literal spec text mandates this; it is a consistency invariant (stated as such rather than inventing a citation): the result schema is a property of the query, not of the number of bound rows, so a DBAPI executemany with an empty parameter list must not see spurious rows or a different schema.
  • On SQLite: fails without the included fix (format, flags and n_children all mismatch at the schema root — see the segfault section below), passes with it. Full suite: 123 passed, 0 failed.
  • On PostgreSQL: passes as-is, including the deep schema comparison (verified against a live PostgreSQL 18 server; the driver falls back to DescribePrepared() when the bind stream produces no result).
  • On Flight SQL (SQLite example server): opts out via a supports_bind_zero_rows() quirk (default true). The arrow-go client fix fix(flight/flightsql): send bound parameters even when binding has zero rows arrow-go#3 gets the zero-rows half right (the binding is sent, the server returns zero rows instead of a stale row; verified locally against that branch via a replace in go/adbc/go.mod) — but the schema half still fails the deep comparison: without any bound values the example server cannot infer concrete result column types and reports SQLite's dense-union fallback (+ud:0,1,2) where an execution with a bound int64 row reports the concrete type (l). Against arrow-go v18.6.0 it fails earlier (missing argument with index 1: the client silently skips the DoPut). Dremio and DuckDB skip via supports_dynamic_parameter_binding().

The SQLite fix also fixes a segfault from Python

The wrong-schema bug is not benign: consumed from Python, it crashes the process (first reported against the released adbc-driver-sqlite 1.11.0 wheel in fornwall/validation#7, whose statement_reader.c is identical to current main). Reproduced with pyarrow 25.0.0 and a driver built from main:

stmt.set_sql_query("SELECT ?")
stmt.prepare()
stmt.bind(pyarrow.record_batch([pyarrow.array([], type=pyarrow.int64())], names=["p0"]))
stream, _ = stmt.execute_query()
pyarrow.RecordBatchReader._import_from_c(stream.address)  # SIGSEGV without this fix

Mechanism:

  1. In the zero-row bind path, InternalAdbcSqliteExportReader sets reader->done before ever stepping the statement and (pre-fix) skipped InferFinalize, leaving reader->schema as the all-zeroes struct it was memset to — in particular format == NULL.
  2. The stream's get_schema deep-copies that struct with nanoarrow's ArrowSchemaDeepCopy, which does not reject the invalid input and installs a real release callback on the copy. The result crosses the C Data Interface boundary looking like a live schema with a NULL format. The Arrow C Data Interface spec is explicit that ArrowSchema.format is "Mandatory. A null-terminated, UTF8-encoded string describing the data type" — only a released structure (indicated by release == NULL) may leave it unset, so a live structure with a NULL format violates the producer's side of the contract, and consumers are entitled to assume it is present.
  3. Arrow C++'s importer only guards against released structs (SchemaImporter::Import, bridge.cc:996-999), so the check passes, and ProcessFormat(), bridge.cc:1091-1092 constructs FormatStringParser(c_struct_->format) — an implicit std::string_view(const char*) conversion that calls strlen(NULL): the segfault.

Full Python call chain: ImportRecordBatchReader (bridge.cc:2509)ArrayStreamBatchReader::ReadSchema (bridge.cc:2306-2311)ImportSchemaProcessFormat. The C++ validation suite's nanoarrow-based reader tolerates the same schema (reading n_children and the format pointers is safe; only parsing the NULL format crashes), which is why the gtest fails cleanly with format/flags/n_children mismatches rather than crashing.

The fault is the driver's (the producer violates the C Data Interface contract); a NULL-format check in Arrow C++'s ProcessFormat() would be worthwhile hardening upstream, but this fix removes the root cause: with InferFinalize running on the zero-row path, the reader reports the statement's real result schema and the Python snippet above prints schema: ?: int64, num_rows: 0.

The test is written against the generic DriverQuirks fixtures, so it auto-enrolls for every driver using ADBCV_TEST_STATEMENT; candidate for later upstream submission to apache/arrow-adbc.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JA2PHSMV54nGRReD9zgfTT

…und parameter stream

When a query was executed with a bound parameter stream containing zero
rows, the reader skipped schema inference entirely and reported an empty
(zero-column) schema instead of the statement's actual result schema.
Run InferFinalize even when the binder finishes before the first
execution, so the schema has the correct number of columns (matching the
behavior of an ordinary query returning zero rows).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq
@fornwall
fornwall force-pushed the validation/bind-zero-rows branch 2 times, most recently from ae84d5f to bc4d82a Compare July 15, 2026 08:39
…ult schema

Executing a parameterized query with a bound parameter batch of zero
rows (e.g. a DBAPI executemany with an empty parameter list) must
return a result stream with zero rows and exactly the same schema as a
non-empty execution of the same query - not execute the query with
stale (or missing) parameters, and not report a placeholder schema.
The schema is compared deeply (format, name, flags, children,
dictionary) against a reference execution with one bound row.

Without the preceding SQLite reader fix this catches the zero-column
placeholder schema (format/flags/n_children all mismatch at the root),
the same malformed schema that segfaults pyarrow when imported from
Python. PostgreSQL passes as-is (verified against a live server).

Add a supports_bind_zero_rows() quirk (default true) so drivers that
cannot satisfy this can opt out, and opt the Flight SQL SQLite tests
out: even with the arrow-go client fix that sends zero-row bindings
(fornwall/arrow-go#3), the example server
cannot infer concrete result column types without any bound values and
reports SQLite's dense-union fallback ("+ud:0,1,2") where an execution
with a bound row reports the concrete type ("l").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JA2PHSMV54nGRReD9zgfTT
@fornwall
fornwall force-pushed the validation/bind-zero-rows branch from bc4d82a to c9e3fa6 Compare July 15, 2026 09:16
Nightly clippy lints can break CI without any code changes on our side.

Recently nightly clippy panicked:

https://github.com/apache/arrow-adbc/actions/runs/29331390016/job/87079916638?pr=4514

Mark the step as `continue-on-error` so failures are visible but
non-blocking.

Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
@fornwall fornwall changed the title test(c/validation): assert a zero-row bound batch still yields the real result schema test(c/validation): assert a zero-row bound batch still yields a result schema Jul 15, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SLERSYbuLBv36tzs7u2Mim
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.

1 participant