Repository navigation
[fix](flight-sql) Support prepared query parameter binding on branch-4.1 - #68768
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Static review complete at a635a61. Two new P1 issues are posted inline: parameterized SELECTs advertise no result schema and are routed as updates by the managed Arrow JDBC client; every advertised parameter type is Arrow Null, so that client rejects non-null binds. There were no existing inline blockers, so existing_blocking_comment_ids is empty. These issues warrant changes.
Critical checkpoint conclusions:
- Goal and proof: the new code converts a single Arrow parameter row, stores typed literals, reparses on execution, and adds direct Flight client tests. It does not deliver usable prepared query binding through the managed Arrow JDBC client because of the two inline issues. No build or test was run in this static-only review; added tests are inspected coverage, not executed proof.
- Scope and clarity: the binding change spans the expected FE parser, session, producer and tests. The POM edits remove duplicate declarations and retain their surviving versions/exclusions; they do not add a separate correctness concern.
- Concurrency: the connection monitor serializes execution and handle close; the prepared-state lock protects map and budget updates; binding versions reject superseded concurrent uploads. I found no supported deadlock or stale-binding interleaving after tracing those call paths.
- Lifecycle and static initialization: handle removal and session teardown release retained bindings and budget; a closed session cannot register new handles, and failed schema matching cancels/reset its result. No new cross-translation-unit initialization or reference-cycle path applies.
- Configuration: no new configuration item is introduced. Existing session settings are re-evaluated during schema analysis; no dynamic-config propagation issue was substantiated.
- Compatibility: Arrow JDBC 19 result classification and bind-vector construction fail as described inline. FE-to-BE wire fields and storage formats are unchanged; SQL-only forwarding rejects typed bindings explicitly instead of dropping them.
- Parallel paths: zero-parameter prepared queries retain the prior command/query path; parameterized query upload and execution were traced. Prepared update upload is still unsupported and is outside this query-binding feature.
- Conditional checks: namespace, handle identity, parameter count, one-row upload, supported vector types, finite floats, UTF-8, and schema recheck have explicit error paths; no further incorrect branch was substantiated.
- Test coverage: the added FE tests and regression cover direct Arrow vector uploads, rebinding, invalid values, schema checks and close races. They bypass the JDBC binder and result classifier, leaving both inline failures uncovered.
- Test results: no generated regression .out files or changed expected-result files are present. Build and test execution was prohibited, so no runtime pass claim is made.
- Observability: upload and schema errors return descriptive Flight statuses; existing query/error logging remains available. No separate metric or logging defect was substantiated.
- Transaction and persistence: no EditLog, durable metadata, transaction commit, or publish logic changes.
- Data writes: no table write path is added; parameter binding only affects query execution. The session byte limit and cleanup were statically checked.
- FE-BE variables: no new transmitted variable is added, so no alternate FE-to-BE send path requires an update.
- Performance and other risks: conversion enforces one-row and byte limits, and session retention has a 16 MiB estimate. Reanalysis occurs at bind/schema/execute for freshness; no additional concrete performance bug was substantiated. Arrow DECIMAL256 and timezone-bearing timestamp vectors are rejected as unsupported; they do not silently misconvert as two investigated candidates had suggested.
Review process: the main risk scan covered all 12 changed files. Round 1 normal and risk passes produced four candidates; main validation accepted the two inline issues and dismissed two with Arrow Java 19 type-mapping evidence. All round 2 normal and risk passes returned NO_NEW_VALUABLE_FINDINGS. The final changed-file sweep and live PR check found no unresolved candidate, no existing inline thread, and an unchanged base/head. User focus: no additional focus was provided.
|
Codex automated review failed and did not complete. Error: Review recovery stopped: No final review submission was declared Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review complete at head 5690018. I reviewed all 13 changed paths, the connected planner/Flight SQL/session code, both existing inline threads, and the added tests. One new P2 is inline below. Existing P1 comment 4214747904 still applies; comment 4214747898 is resolved on this head. No additional user focus was supplied.
Goal and scope: The change wires single-row Arrow parameter upload into fresh Nereids query parsing and execution and advertises known CAST/comparison parameter and result types. It does not fully achieve bindable JDBC metadata for fixed-type LIKE, IN, and BETWEEN operands: those still advertise Arrow Null, so the existing P1 remains blocking. The implementation is mostly focused across the necessary FE layers; the two POM edits remove duplicate dependencies.
Concurrency and lifecycle: Prepare, GetSchema, execution, and handle close use the connection monitor. DoPut reads its stream outside that monitor and checks a binding version before commit; overlapping uploads and close cannot commit stale values. Session teardown invalidates handles under the prepared-state lock, and the execution path rejects a ticket after teardown. Parameter retention has per-upload and per-session byte limits and releases charges on rebind/remove/clear. The new P2 concerns the separate result-schema baseline lifecycle. No new lock-order deadlock or static-initialization issue was substantiated.
Configuration, compatibility, and parallel paths: No config, storage format, EditLog, FE-BE variable, or wire message is added. Session sql_mode/time_zone and concurrent DDL can affect schema analysis; the inline P2 identifies the missing baseline check across Prepare, unbound GetSchema, and DoPut. Bound Flight queries are explicitly rejected before SQL-only master forwarding, avoiding lost typed values. Ordinary statement execution remains a separate path. Unsupported parameter vector types, wrong counts, excess rows, invalid values, and memory excess return errors instead of silently executing.
Tests and operations: I inspected the FE unit tests and ordered Arrow Flight SQL regression, including JDBC metadata, direct Flight binds, rebind/null/error cases, conversion precision, and teardown. They were not executed because this review runner prohibits builds and tests; inspected tests and expected results are not independent validation. The existing errors/logging cover the new rejection paths; no additional metric need or hot-path performance bug was substantiated. No transaction, persistence, or data-write path changes are involved. The final changed-file and unresolved-candidate sweep found no other substantiated new issue.
Existing P0/P1 findings confirmed for this head: #68768 (comment)
| query.parameterBytes = bytes; | ||
| // Retain detached immutable values, never vectors owned and reused by the Flight stream. | ||
| query.parameters = parameters == null ? null : List.copyOf(parameters); | ||
| query.schema = schema; |
There was a problem hiding this comment.
[P2] Preserve the Prepare schema separately from the bound schema. For SELECT * FROM t WHERE id = ?, a completed ALTER TABLE t ADD COLUMN between Prepare and DoPut makes analyzeQuerySchema return a wider result, and this assignment replaces the original baseline. preparedQuery then accepts a handle whose result shape changed despite the existing schema-drift guard. Unbound GetSchema also returns the changed shape without checking the baseline. Reject incompatible DDL or session-setting drift in both paths while allowing fields whose type genuinely depends on a bound value.
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review of PR #68768 at d97c90c. All 13 changed files were inspected. The normal protocol/session pass, normal planner/conversion pass, and separate risk-focused pass converged after one round with no new valuable findings; the final changed-file and candidate sweep found no unresolved point.
Existing findings: P1 inline comment 4214747904 remains applicable in part. CAST and simple comparison placeholders now advertise bindable types, but fixed types in IN (?) and BETWEEN ? AND ? still fall back to Arrow Null, so Arrow JDBC cannot bind non-null values. This is the same bindability concern and is not reposted. P2 inline comment 4215155737 still applies: DoPut replaces the Prepare result-schema baseline after reanalysis, and unbound GetSchema does not compare against it. It is not a P0/P1 blocker ID. The earlier P1 comment 4214747898 about an empty dataset schema for a known SELECT result is addressed by Prepare-time planning and schema publication.
Critical checkpoints:
- Goal and tests: The implementation provides one-row prepared-query binding, typed metadata for supported cases, rebind handling, and execution through FlightInfo/DoGet. The Java and regression tests inspect direct Flight, JDBC, invalid uploads, and lifecycle cases. They do not prove the remaining IN/BETWEEN JDBC paths; the feature is incomplete for those clients.
- Scope and conditions: Changes are concentrated in Flight SQL preparation, parsing, session state, type conversion, and the associated tests; two POM edits remove duplicate declarations. One-row and unsupported-type conditions return explicit errors. No additional parallel-path or conditional bug was substantiated.
- Concurrency and lifecycle: Prepare, schema lookup, execute, and handle close serialize on the connection monitor. DoPut reads outside that monitor and uses the prepared-state lock and binding version before commit; session teardown invalidates handles. The inspected lock order has no reversed acquisition or established deadlock. Full query execution holds the connection monitor to protect mutable session state. No new cross-translation-unit static initialization issue was found.
- Configuration and compatibility: No configuration item, FE-to-BE variable, storage format, function symbol, or mixed-version wire field is added. SQL-only forwarding rejects a bound Flight query before its values could be lost. Ordinary unprepared Flight queries retain their existing path.
- Data, errors, and observability: No new transaction, persistent metadata, or data-write path is introduced. Invalid shapes, unsupported values, expired handles, and superseded binds fail explicitly. Existing query error logging and statuses provide context; no new metric need was substantiated.
- Performance and validation: Prepare/Bind/Execute reanalysis has cost but serves schema validation, and no independently confirmed avoidable hot-path defect was found. No builds or tests were run under this static-only review instruction. The added regression assertions were inspected; no generated result file changed.
- User focus: No additional focus was provided.
Existing P0/P1 findings confirmed for this head: #68768 (comment)
|
run buildall |
|
/review |
FE UT Coverage ReportIncrement line coverage |
There was a problem hiding this comment.
Complete static review of apache/doris PR #68768 at head 459eb89. Two full convergence rounds, a separate risk pass in each round, existing-thread deduplication, and a final sweep of all 14 changed paths found no distinct new inline issue. No additional user review focus was specified.
Existing review context: P2 inline comment 4215155737 still applies. DoPut stores the newly analyzed bound schema in place of the Prepare baseline, so a SELECT * handle can accept a changed table shape after DDL, and unbound GetSchema can return that changed shape. This is already reported and is not reposted. The two earlier P1 comments (4214747898 and 4214747904) are resolved for their stated SELECT CAST(? AS BIGINT) case: Prepare now advertises the result schema and bindable BIGINT parameter metadata, with JDBC coverage. No existing P0/P1 inline blocker was independently confirmed to remain on this head.
Goal and scope: The change supports a single row of typed Flight SQL prepared-query values, advertises types inferred from explicit casts and comparison columns, binds detached literals into a newly parsed statement, and rejects unsupported/multiple-row uploads. Those paths match the stated supported-scalar scope, subject to the existing P2 schema-drift issue. The FE changes form one prepare/bind/execute flow; the two duplicate Maven declaration removals are narrow CI model cleanup.
Concurrency and lifecycle: DoPut uploads, prepared execution, handle close, and session teardown can overlap. The per-handle binding version and prepared-map lock reject stale upload commits; normal handle close serializes with execution on the connection monitor; session close clears handles and the post-execution check cancels results for an invalidated handle. Immutable parameter snapshots, byte accounting, rebind invalidation, deferred-executor cleanup, and channel teardown were inspected. I found no additional proven lock-order, stale-binding, leak, or static-initialization issue.
Configuration, compatibility, and parallel paths: No configuration item or persisted format is added. Bound parameters use existing planner expressions rather than a new FE-to-BE field; forwarding a bound Flight query to the master is rejected explicitly because that path carries SQL text but not bindings. Both direct Flight and JDBC prepared paths were reviewed. The one-statement/one-row/type/size checks fail explicitly, and unsupported vector types are outside the PR's stated bindable set.
Correctness, safety, and performance: Placeholder order is taken from source-token order, and execution reparses and binds a fresh statement context. Query schema analysis checks privileges; actual execution schema is compared before publishing a ticket. Supported numeric, UTF-8, decimal, date, timestamp, and typed-null conversions were inspected, as were memory limits and the existing per-session execution serialization. No additional supported-value corruption or avoidable high-impact hot-path regression was substantiated. Prepared parameter queries are restricted to non-Command query plans, so this patch adds no transaction, data-write, or EditLog path.
Tests and observability: Changed FE tests and the regression suite inspect JDBC metadata/execution, direct Flight binding/rebinding, NULLs, invalid counts and uploads, session close, and result cleanup. Their expected outcomes were statically reviewed; this review did not run builds or tests. The PR author reports passing validation, which is not independent review execution. Existing Flight statuses/logging provide the inspected failure paths; no new metric requirement was substantiated.
Review completion: Every initial risk item and candidate has a recorded disposition in the shared ledger. No new comments are proposed; the still-applicable P2 should be resolved in its existing thread before merge.
459eb89 to
7d20875
Compare
|
run buildall |
|
/review |
There was a problem hiding this comment.
Review complete at head 7d20875 after two full-review rounds, a focused risk scan, candidate deduplication, and a final sweep of all 13 changed paths. This was static inspection only; the review workflow prohibited builds and tests.
Goal and result: The PR implements one-row typed Arrow Flight SQL prepared-query binding through Prepare metadata, DoPut conversion, fresh Nereids placeholder binding, and execution schema checks. Explicit CAST and direct comparison examples have result and parameter metadata. A timezone-aware TIMESTAMPTZ binding can still change the uploaded instant (new inline P2). Two existing concerns remain: fixed-type JDBC binding for shapes such as id IN (?) (existing P1 4214747904), and preservation of the original Prepare schema across binding/DDL (existing P2 4215155737). These are not reposted. The old empty Prepare result-schema P1 4214747898 is resolved for its concrete CAST example.
Scope and tests: The implementation is focused on prepared query parameters; the pom.xml edit removes a duplicate property without changing its effective value. The added FE tests and Flight SQL regression cover typed CASTs, direct range predicates, repeated execution, NULL, invalid uploads, close races, and JDBC metadata/binding. They were inspected, not executed here. The missing non-UTC TIMESTAMPTZ case accompanies the new inline finding; an IN (?) JDBC bind would exercise the still-applicable P1.
Concurrency and lifecycle: Flight RPC upload, query execution, handle close, and session teardown can overlap. Binding versions and the prepared-state lock reject stale uploads; the connection monitor serializes execution with per-handle close while stream reading occurs outside it. Session teardown clears prepared state and closes channel/deferred results. No additional deadlock, stale commit, or orphaned result was substantiated. There is no new cross-translation-unit static initialization concern in this Java change.
Configuration and compatibility: No dynamic configuration or storage format is added. The new client-visible parameter and result schemas are prepared before binding. Unsupported vector types and multirow uploads fail explicitly. Bound queries that require forwarding to the master FE are rejected because the SQL-only forwarding transport cannot carry typed values; this covers the parallel FE path. No new FE-to-BE variable, transaction, EditLog, data-write, or cloud visibility path is introduced.
Conditions, performance, and observability: The single-row and type restrictions are explicit. Per-upload and per-session parameter limits bound retained values; no distinct unbounded-memory or hot-path regression was substantiated. Existing Flight statuses and query identifiers cover the new error paths; no required new metric was identified. Privilege checks and final execution-schema comparison were traced. The original Prepare baseline remains the existing P2 concern.
User focus: No additional review focus was provided. The complete changed-file and unresolved-candidate sweep found no other new issue. Round 2 subagents returned NO_NEW_VALUABLE_FINDINGS.
Existing P0/P1 findings confirmed for this head: #68768 (comment)
| LocalDateTime time = LocalDateTime.ofEpochSecond( | ||
| Math.floorDiv(timestamp, units), (int) nanos, ZoneOffset.UTC); | ||
| checkYear(time.getYear()); | ||
| return new DateTimeV2Literal((DateTimeV2Type) type, time.getYear(), time.getMonthValue(), |
There was a problem hiding this comment.
[P2] Preserve timezone-aware Arrow timestamp instants when binding. For a non-UTC session, SELECT CAST(? AS TIMESTAMPTZ) advertises a timezone-bearing Arrow timestamp, whose integer is already relative to the UTC epoch. This branch makes UTC wall-clock fields into a DateTimeV2Literal; the subsequent DATETIMEV2-to-TIMESTAMPTZ cast interprets those fields in the session timezone and shifts the instant again. For example, epoch zero in an Asia/Shanghai session becomes 1969-12-31 16:00:00 UTC, so TIMESTAMPTZ predicates can miss rows. Keep timezone-bearing uploads as an instant (or explicitly convert from UTC) and test a non-UTC session.
|
run buildall |
|
/review |
FE UT Coverage ReportIncrement line coverage |
There was a problem hiding this comment.
Completed static review of PR #68768 at head a19b986aefde48c00a21faf0037801c28da9e9c9: all 13 changed files, upstream/downstream paths, existing threads, three bounded review rounds, and a final changed-file sweep. Two distinct P2 issues are inline below: determinable input types still advertised as Arrow Null outside direct casts/comparisons, and stale advertised parameter types after DDL that preserves the output shape.
Existing thread r4215155737 remains applicable: DoPut can replace the result schema promised by Prepare, and unbound GetSchema does not check the original baseline. It is already reported, so I did not duplicate it. Earlier P1 threads r4214747898 and r4214747904 have their direct CAST(? AS BIGINT) result and parameter-schema examples fixed on this head; thread r4217619395's timezone-aware timestamp example is also fixed. No previously reported P0/P1 inline finding was independently confirmed still applicable to this head.
Critical checkpoints:
- Goal and proof: the code implements single-row typed Flight SQL query binding through Prepare, DoPut, and fresh Nereids execution for explicit casts and direct column comparisons. The added FE/JDBC and Groovy tests cover those paths, repeated binds, NULLs, invalid uploads, forwarding, and timezone cases. M1 and M2 show remaining correctness gaps; no test covers those scenarios.
- Scope and clarity: the changed production paths are concentrated in Flight SQL, its Nereids placeholder hook, session state, and the forwarding guard. The removed duplicate
nimbusds.versiondeclaration leaves the effective later version and dependency reference in place. - Concurrency and locks: Flight worker uploads can overlap execution, handle close, and session teardown. Execution and handle close serialize on the connection monitor; short prepared-state operations use their own lock and upload versions reject stale commits. I found no new deadlock or stale-binding publication, and schema analysis/stream reads are outside the short prepared-state lock.
- Lifecycle: rebind invalidates old values, close/timeout clears handles, and failed prepared execution cancels/cleans deferred executors and Arrow results before publishing a ticket. No cross-translation-unit static initialization issue applies to this Java-only change.
- Configuration and compatibility: no new configuration, storage format, EditLog record, or FE-to-BE variable is added. Bound queries that require SQL-only forwarding to a master FE fail explicitly instead of dropping their values. Direct Flight and JDBC query paths were both reviewed; prepared update is outside this query feature.
- Conditions and parallel cases: count, upload size, one-row, unsupported-type, invalid-handle, schema and forwarding checks report failures. M1 identifies additional typed expression forms missed by inference; M2 identifies an input-schema change missed by the schema guard.
- Tests and expected results: the new FE tests and Groovy regression have relevant positive and negative cases, including ordered range outputs and real JDBC/direct Flight calls. The M1 operator shapes and M2 DDL transition need regression cases. No
.outresult was changed. This review did not run builds or tests; author/CI validation claims were not independently executed here. - Observability: new bind failures use Flight status descriptions and existing query/error logging; no distinct missing metric or diagnostic failure was substantiated.
- Persistence, writes, failover: bound values are session-local query state and do not add transaction writes or durable metadata. There is no new EditLog/replay or commit/publish path. Master forwarding is rejected for bound queries because the existing forwarding payload carries only SQL text.
- Memory and performance: uploads are limited to 1 MiB and retained session parameters to 16 MiB, with detached literals and explicit release on rebind/close. Reanalysis is needed for the schema contract; no separate unbounded hot-path or memory issue was substantiated.
User focus: no additional focus points were supplied. Review completion: complete static review after three rounds; the two new P2 comments and the still-applicable existing P2 describe the remaining issues.
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
…4.1 (#68768) ### What problem does this PR solve? Flight SQL prepared queries containing `?` fail because Prepare rejects placeholders and `acceptPutPreparedStatementQuery` returns `UNIMPLEMENTED`. For example, ADBC cannot bind an integer to `SELECT CAST(? AS BIGINT)` or execute a range predicate with two bound values. This change implements single-row typed parameter binding on branch-4.1. Prepare analyzes explicit casts and comparison columns to advertise parameter types and a best-effort result schema. DoPut converts Arrow values into detached Nereids literals, and execution binds those literals to a fresh statement context. Result schemas are reanalyzed after binding. Values are never interpolated into SQL text. Arrow JDBC receives typed parameter metadata and a result schema before binding, so it can construct parameter vectors and classify SELECT correctly. Types that cannot be inferred remain unknown; JDBC callers should use an explicit CAST for queries such as `SELECT ?`. Timezone-bearing timestamp uploads preserve their UTC instants as TIMESTAMPTZ literals; timestamps without a timezone retain wall-clock semantics. Unsupported parameter vector types are not advertised as bindable. Invalid SQL returns `INVALID_ARGUMENT` consistently through Prepare and GetSchema. Failed rebinds invalidate previous values. Handle ownership, concurrent closure, upload versions, and per-binding/session memory limits protect retained state. Multi-row uploads and unsupported types fail explicitly. Queries requiring forwarding to the master FE reject bound parameters because the forwarding protocol cannot carry typed bindings. The PR also removes an obsolete duplicate JWT version property, preserving the effective dependency version. The target branch already contains the dependency declaration cleanup. ### Release note Support single-row scalar parameter binding for Flight SQL prepared queries, including repeated execution and NULL values. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [x] Manual test - Behavior changed: - [x] Yes. Parameterized Flight SQL queries can bind and execute supported scalar values. - Does this need documentation? - [x] No. This implements the existing prepared-query protocol. Validation: - Full FE Checkstyle passed (`mvn -B clean checkstyle:check`). - Focused compilation of all 11 changed Java source/test files passed with `javac --release 8` using existing FE dependency artifacts. Java 8-compatible Guava copies preserve immutable parameter snapshots. - 23 new tests and 33 existing Flight SQL schema tests passed, including invalid-SQL status checks over Flight RPC and connection recovery. - The new regression suite passed against a local FE service test harness and a real BE, including actual JDBC `PreparedStatement` execution, result fetching, rebinding, NULL, mixed types, range predicates, and recovery after invalid uploads. TIMESTAMPTZ predicates are checked under UTC, Asia/Shanghai and America/New_York, including a daylight-saving transition. - ADBC 1.11.0 with PyArrow 23.0.1 passed 18 concurrent connections and 540 parameterized queries, with result verification plus NULL and connection-reuse checks. A further 72 timestamp result checks cover all four Arrow units, three session timezones, two input timezone annotations, negative epochs and NULL values. The local integration harness uses one FE service and one real BE; a full multi-FE cluster run and full FE build are left to CI. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
…Schema (#69867) ### What problem does this PR solve? Issue Number: None Related PR: #68631, #68768 Problem Summary: **In short.** This ports branch-4.1 #68631 (also on branch-4.2) to master. A Flight SQL `CreatePreparedStatement` now returns the analyzed result schema of the statement, `GetSchema` is implemented for both statements and prepared statements, and a prepared execution is checked against the schema that Prepare advertised. Before this PR, Prepare always returned a fixed one-column placeholder, and `GetSchema` answered UNIMPLEMENTED. The analysis runs in the FE only and does not plan or execute the query. Original author: @Gabriel39 **Background.** Flight SQL clients learn the shape of a result before they fetch it. The Arrow Flight SQL JDBC driver prepares every statement and builds `ResultSetMetaData` from the dataset schema that `CreatePreparedStatement` returns. ADBC exposes the same schema as `execute_schema`, and `GetSchema` is the RPC that asks for it without running anything. **The problem, and what it cost.** On master: | | master today | with this PR | |---|---|---| | `CreatePreparedStatement` dataset schema | always `ResultMeta: Utf8`, whatever the statement | the statement's columns, names and Arrow types as BE / FE will send them; statements whose schema cannot be told before they run, and results with an untyped NULL column, keep the placeholder (see below) | | `GetSchema` for a statement or a prepared statement | UNIMPLEMENTED | the analyzed schema, or UNIMPLEMENTED for a statement that keeps the placeholder | | Invalid SQL | fails only at execution | fails at Prepare with INVALID_ARGUMENT | | A query whose privilege check fails | fails only at execution | still fails only at execution: Prepare keeps the placeholder, because only the check on the executed plan decides (see below), and GetSchema answers UNIMPLEMENTED without a schema | | Prepared handle used after the session's catalog / database changed, or after the result schema changed (e.g. another session's DDL) | runs the unqualified SQL in whatever namespace the session has now, with whatever columns come back | refused with NOT_FOUND "prepare again", and the result left by that execution is dropped | | Prepared handle presented by another session | resolved by the identity written in the handle | must start with the caller's own identity (from #68768) | | TIMEV2 / VARBINARY columns in GetTables and Prepare schemas | `Null`, while BE sends `float64` / `binary` | `Float64` / `Binary`, as BE sends them | A client that trusts the advertised schema saw one string column for every query, and could not ask for a schema at all without executing. **How this PR fixes it.** - `arrowflight/FlightSqlQuerySchema` analyzes the statement on a clone of the session variables: a USE / SWITCH prefix is resolved in a temporary namespace, the analysis `StatementContext` is closed on the same thread, and the connector statement scope it may have opened is closed explicitly. Each field gets the same `doris_type` markers and native Variant layout the execution side uses (`FlightSqlSchemaHelper.withDorisTypeMetadata` / `nativeVariantField`), and TIMESTAMPTZ carries the canonical session time zone that BE stamps on its batches. - Privileges are checked on the analyzed plan after the CTEs that execution inlines have been inlined (`PullUpCteAnchor`, `CTEInline`, as the rewriter does before its own check). A CTE used once is therefore checked only on the columns the query reads, and a CTE used more than once on every column it selects, which is also how execution checks them. The rewriter normalizes the plan further before it checks privileges. For example, it drops an ORDER BY under a derived table, so the analyzed plan can read a column that the executed plan does not. When the analyzed plan is denied, the statement keeps the placeholder and execution decides. No schema is disclosed in either case. - FE-side commands whose result header does not depend on running them now report that header without running: HELP, SHOW SNAPSHOT, SHOW CREATE TABLE (one header selector shared with `doRun`, including master's stream, plugin-view and system-table cases), SHOW PROC `/` and `/current_queries`, SHOW CONFIG, and EXPLAIN PLAN PROCESS (Rule / Before / After). - `ShowStoragePolicyCommand.getMetaData()` returned the four-column USING header for every form, while plain SHOW STORAGE POLICY answers the seven columns of `StoragePolicy.STORAGE_META_DATA`. It now returns the header of the form that runs. This also corrects the column definitions that MySQL `COM_STMT_PREPARE` reports for the statement. No other SHOW command declares a header that differs from the one it returns. - WARM UP SELECT keeps the placeholder. It answers OK instead of its statistics whenever planning proves its source empty: a predicate that folds to false, no partition left after pruning, or partitions without data. - A result with an untyped NULL column keeps the placeholder too, at any depth (`SELECT NULL`, `SELECT id, NULL FROM t`, `ARRAY(NULL)`). `DorisArrowTypeMapping` gives NULL_TYPE Arrow `Null`, but BE builds such a column as a `UInt8` flagged as a NULL literal and sends it as nullable `Bool`, so an advertised `Null` would expire the handle at every execution. A typed NULL (`CAST(NULL AS VARCHAR(20))`) is described as its type. - The prepared query kept in `FlightProtocolAdapter` records the SQL, the namespace and the advertised schema. A prepared execution re-analyzes the statement and compares the result with the advertised schema before it runs, and compares the schema it actually produced before it publishes a DoGet ticket. It is the session's next request whether or not these checks pass: before them, it drops what the previous request left, as a statement's execution does, while Prepare and GetSchema are metadata requests and leave it. On a mismatch, the handle is removed, the execution is cancelled, and the deferred coordinator, the channel result and the endpoints are dropped. A handle whose statement can no longer be described when it is re-analyzed, for example after a column grant was revoked, expires the same way, and the statement prepared again keeps the placeholder. A USE / SWITCH inside the statement rebinds only that handle. - `DorisArrowTypeMapping` maps TIMEV2 to `Float64` and VARBINARY to `Binary`, the types BE sends. TIMESTAMPTZ keeps the fixed `"UTC"` in GetTables, and AGG_STATE keeps `Null` there. - `FeServiceClient` rejects a remote Doris metadata timeout whose milliseconds do not fit in an int, instead of letting it wrap around. Where master differs from branch-4.x: - branch-4.x answers UNIMPLEMENTED at Prepare for statements whose schema it cannot tell before they run: DESC, SHOW DATA / PARTITIONS / QUERY STATS / PYTHON PACKAGES, other PROC nodes, COPY INTO, WARM UP CLUSTER, multi-statement requests that do not start with USE / SWITCH, `?` placeholders, and result types the FE cannot describe. Python ADBC falls back to executing without a prepared statement, but the Arrow Flight SQL JDBC driver prepares every statement and has no fallback, so on master these statements would fail over JDBC. `test_arrow_flight_session_lifecycle` runs DESC and `SET @multi = 2; SELECT ...` through JDBC. Here, these statements keep today's placeholder schema, and their handles are not checked at execution. - branch-4.x checks privileges on the analyzed plan as it is, so a CTE used once is checked on every column it selects, and any denial fails Prepare. Here, CTEs are inlined first, and a remaining denial keeps the placeholder. - branch-4.x advertises the statistics header for WARM UP SELECT and then rejects, as a schema change, an execution whose source turned out empty. Here, WARM UP SELECT keeps the placeholder. - branch-4.x advertises Arrow `Null` for an untyped NULL column, which BE sends as `Bool`, so every prepared execution of such a query is rejected as a schema change. Here, such a result keeps the placeholder. - The analysis and the checks run under the session's command lock (`FlightProtocolAdapter.runCommand` / `callCommand`) instead of `synchronized (connectContext)`, and the namespace is saved and restored with `getNameSpaceContext()` / `restoreNameSpaceContext()`. - EXPLAIN PLAN PROCESS has a result over Flight on master, so it gets a static header instead of UNIMPLEMENTED. PLAN REPLAYER and SHOW CREATE PROCEDURE do not exist on master and are not carried over. - SWITCH resolves the default database through the connector provider (`Env.defaultDatabaseOnUse`, now shared with `Env.changeCatalog`) instead of `EsExternalCatalog.DEFAULT_DB`. - TIMESTAMPTZ fields carry the canonical session time zone that BE stamps, where branch-4.x uses the raw session value. - Only the two handle checks from #68768 that do not depend on parameter binding are taken: the caller-prefix check and rebinding only the handle that ran the USE / SWITCH. Parameter binding itself is not part of this PR. - `GetTableTypes`, the other half of #68631, is implemented by #67899. Cost: every Prepare, every prepared execution and every `GetSchema` runs one more FE analysis of the statement. **Results.** On a local cluster built from this branch, every suite under `arrow_flight_sql_p0` passes: the new `test_schema_only` and `test_get_tables_schema`, plus the seven suites that run through Flight JDBC, `test_arrow_flight_session_lifecycle` among them. `datatype_p0/uuid/test_uuid_arrow_flight` passes too. `test_schema_only` checks the advertised schema of each kind of statement against what its execution actually returns. That covers queries, the side-effect-free frontend-side commands executed as prepared statements (SHOW STORAGE POLICY among them), a WARM UP SELECT on an empty table (executed in cloud mode, the only mode that warms up internal tables), and a `group_concat_state` AGG_STATE, which BE sends as Utf8. Co-authored-by: Gabriel <liwenqiang@selectdb.com>
What problem does this PR solve?
Flight SQL prepared queries containing
?fail because Prepare rejects placeholders andacceptPutPreparedStatementQueryreturnsUNIMPLEMENTED. For example, ADBC cannot bind an integer toSELECT CAST(? AS BIGINT)or execute a range predicate with two bound values.This change implements single-row typed parameter binding on branch-4.1. Prepare analyzes explicit casts and comparison columns to advertise parameter types and a best-effort result schema. DoPut converts Arrow values into detached Nereids literals, and execution binds those literals to a fresh statement context. Result schemas are reanalyzed after binding. Values are never interpolated into SQL text.
Arrow JDBC receives typed parameter metadata and a result schema before binding, so it can construct parameter vectors and classify SELECT correctly. Types that cannot be inferred remain unknown; JDBC callers should use an explicit CAST for queries such as
SELECT ?. Timezone-bearing timestamp uploads preserve their UTC instants as TIMESTAMPTZ literals; timestamps without a timezone retain wall-clock semantics. Unsupported parameter vector types are not advertised as bindable. Invalid SQL returnsINVALID_ARGUMENTconsistently through Prepare and GetSchema.Failed rebinds invalidate previous values. Handle ownership, concurrent closure, upload versions, and per-binding/session memory limits protect retained state. Multi-row uploads and unsupported types fail explicitly. Queries requiring forwarding to the master FE reject bound parameters because the forwarding protocol cannot carry typed bindings.
The PR also removes an obsolete duplicate JWT version property, preserving the effective dependency version. The target branch already contains the dependency declaration cleanup.
Release note
Support single-row scalar parameter binding for Flight SQL prepared queries, including repeated execution and NULL values.
Check List (For Author)
Validation:
mvn -B clean checkstyle:check).javac --release 8using existing FE dependency artifacts. Java 8-compatible Guava copies preserve immutable parameter snapshots.PreparedStatementexecution, result fetching, rebinding, NULL, mixed types, range predicates, and recovery after invalid uploads. TIMESTAMPTZ predicates are checked under UTC, Asia/Shanghai and America/New_York, including a daylight-saving transition.The local integration harness uses one FE service and one real BE; a full multi-FE cluster run and full FE build are left to CI.
Check List (For Reviewer who merge this PR)