Repository navigation
[fix](flight-sql) Return analyzed query schemas and table type metadata - #68631
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
|
run buildall |
|
/review |
|
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. |
|
/review |
There was a problem hiding this comment.
Complete static review of PR #68631 at d117502. Three review rounds covered all eight changed files; the final normal and risk-focused reviews returned NO_NEW_VALUABLE_FINDINGS. I recommend changes for the one P1 and eight P2 inline findings. There were no existing inline threads or previously reported P0/P1 findings to carry forward. No additional user focus was supplied.
Critical checkpoints
- Goal and proof: The PR replaces placeholder Flight SQL prepare metadata and implements direct/prepared GetSchema through FE analysis. The new FE and Groovy tests cover basic SELECT, nested types, SHOW VARIABLES, invalid SQL, privilege denial, SET/SET_VAR isolation, and allocator cleanup. The nine inline cases show the goal is incomplete for supported dialects, commands, session transitions, and aggregate-state output.
- Scope and clarity: The eight-file change is focused on schema discovery, but the new resolver duplicates execution's dialect and namespace behavior (M01, M02) and assumes command metadata is ready before command validation (M03, M04, M07, M08).
- Concurrency: Flight RPCs reuse a mutable
ConnectContext. Schema analysis takes its monitor while ordinary execution does not; a concurrent successful SET can be discarded when analysis restores its saved session variable (M05). I found no separate new lock-order or deadlock issue. - Lifecycle and errors: The resolver restores statement, thread, and session references in
finally, and the moved allocator test checks repeated successful prepares. Preparation now rejects executable SHOW commands with empty static metadata (M08), and the embedded prepared schema can diverge after USE (M09). SHOW PARTITIONS and SHOW QUERY STATS fail before metadata setup; the wrapper reports those failures as invalid schema requests (M07). - Configuration and compatibility: No new configuration or persistent format is added. Existing SQL dialect conversion is skipped by schema discovery (M01). The prepared result schema and GetSchema/FlightInfo paths must agree with existing Flight clients; M03, M04, M06, M08, and M09 show concrete disagreements. No new FE-to-BE session variable or wire field is added.
- Parallel paths and conditions: I compared direct GetSchema, prepared GetSchema, Prepare, GetFlightInfo, FE-local result sets, and BE Arrow schema construction. The command
stmtType/empty-metadata fallback and the ShowTable-only validation special case are insufficient for the cited commands (M03, M04, M07, M08). Ordinary scalar, date/time, binary, and nested Arrow mappings match the inspected BE path; AGG_STATE serialization is the confirmed exception (M06). - Tests and results: The new tests assert baseline positive and negative cases but do not exercise the reported command shapes, aggregate state, configured dialect conversion, remembered catalog database, concurrent SET, or post-prepare USE. No
.outresult file changed. This runner's review instructions prohibit builds and tests, so this is static source evidence only; author validation claims were not used as independent proof. - Observability, persistence, and writes: No new EditLog, transaction, data-write, or crash-recovery path is introduced by schema lookup. The resolver returns Flight errors; M07's internal null-state failure is currently surfaced as INVALID_ARGUMENT. I found no separate log or metric defect supported by this diff.
- Performance and remaining issues: Prepare and GetSchema add an FE analysis pass; the PR description acknowledges that cost. I found no separate measurable hot-path regression. The final changed-file sweep left no unresolved candidate or risk item; all substantiated defects are in the inline set.
|
run buildall |
|
/review |
|
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. |
|
/review |
There was a problem hiding this comment.
Review status: INCOMPLETE under the required three-round convergence cap. Seven distinct [P2] findings are attached. The final round identified the EXPLAIN/REPLAY preparation regression, so the all-NO_NEW_VALUABLE_FINDINGS condition was not reached. All identified candidates were independently checked and deduplicated before submission. Static review only; no build or test was run.
Critical checkpoints:
- Goal and proof: The change derives Flight SQL schemas without executing statements. New FE and Flight RPC tests cover ordinary queries, selected SHOW commands, namespace changes, Arrow types, and error paths, but they do not cover the seven cases reported here; this review did not run them.
- Scope: The 12 changed paths are focused on schema discovery and its producer/command support. The command metadata classifier still misses valid result and protocol OK shapes.
- Concurrency and lifecycle: Schema calls, normal execution, and prepared execution use the per-session monitor; the earlier P1 lost-SET race is fixed. Scoped analysis restores session, statement, executor, state, namespace, and thread context. SHOW PROC now fetches rows under that monitor (M3). No separate lock-order or resource-release defect was substantiated.
- Configuration: No new setting is introduced. Existing sql_mode changes can stale a prepared handle within one namespace (M1), and the existing dialect parse-retry setting is omitted from discovery (M2).
- Compatibility and parallel paths: No storage or wire-format change was found. Direct Flight execution and GetSchema/Prepare differ for Lance index ALTER, procedure SHOW, ordinary no-row commands, and EXPLAIN/REPLAY (M4-M7). The classifier must use actual command result shapes; WARM UP SELECT shows why a blanket INSERT rule is unsafe.
- Conditions and tests: Existing negative tests cover some unsupported dynamic SHOW and OTHER commands, but not the reported variants or EXPLAIN/REPLAY. The changed test assertions were inspected as source; their runtime results were not independently verified.
- Observability and performance: Existing error propagation exposes rejected schema calls; no separate logging or metric issue was substantiated. SHOW PROC's full fetch is the concrete avoidable CPU, memory, and possible BE RPC work (M3).
- Persistence, transactions, and writes: The diff adds no EditLog, persistence, or write implementation. It does change preparation compatibility for ordinary DML and BEGIN (M6); no transaction atomicity issue was found in the unchanged execution path.
- FE/BE variables and other correctness: No new FE-to-BE variable is transmitted. Arrow field mapping was compared with BE and FE-local result construction without another independently substantiated type mismatch. All changed files and nine existing inline threads were checked; no existing P0/P1 finding remains applicable. The user supplied no additional review focus.
|
run buildall |
|
/review |
|
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. |
There was a problem hiding this comment.
COMPLETE static review of PR #68631 at head 37f71c9. Three convergence rounds finished; all final-round subagents returned NO_NEW_VALUABLE_FINDINGS. Four distinct P2 findings are inline. No P0/P1 remains on this head: the earlier P1 session-clone race (comment 4131745680) is addressed by the shared ConnectContext monitor around schema analysis and ordinary/prepared execution. Existing lower-priority threads were treated as duplicate fences.
Critical checkpoints:
- Goal and scope: GetSchema and Prepare now derive FE/BE-compatible headers without constructing StmtExecutor or dispatching query fragments. The analyzer uses Nereids output slots and privilege checks for SELECT, and explicit command metadata for FE results. The change is focused on Flight schema discovery and the command interfaces it needs, although the resolver must cover many command shapes. HELP and filtered SHOW SNAPSHOT remain incorrect (inline findings).
- Concurrency and lifecycle: Same-session schema analysis, Prepare registration, and GetFlightInfo execution share the session monitor. Analysis closes parsed statement contexts and restores the prior session variable, query state, executor, namespace, and thread context. No new lock-order cycle, static-initialization dependency, or unclosed Arrow allocation was established. Cross-session DDL can still change a prepared query schema between reanalysis and execution, and a prepared USE/SWITCH can invalidate its own handle (inline findings). Deferred-executor cleanup was checked on success/failure paths.
- Configuration and compatibility: No new config item, FE-to-BE variable, function symbol, storage format, or persisted protocol is added. Result-affecting session settings are reanalyzed before prepared use; namespace/command compatibility gaps are described inline. Direct statement, prepared statement, FE-local result, BE query result, cloud/non-cloud, and explicit unsupported-command paths were considered.
- Conditional paths and errors: Known execution-dependent SHOW metadata is declined; SHOW SNAPSHOT's WHERE-dependent header is still read before validation. Command errors are converted at the Flight boundary. Unsupported parameter binding and arbitrary multi-statement batches are explicitly reported. No separate silent-status or observability defect was substantiated.
- Tests and results: The new FE tests and Flight regression suite cover normal SELECT/SHOW/DML schemas, nested and Arrow types, privileges, dialect fallback, invalid SQL, session isolation, and namespace/schema drift. They do not cover the four inline cases, and Prepare/GetSchema agreement alone does not prove execution parity. No .out file or expected-result change was added. Builds and tests were prohibited for this review, so these conclusions are static and author/CI claims are not independent validation.
- Persistence, writes, performance, and remaining invariants: Schema discovery adds FE analysis but does not itself commit transactions, mutate table data, write EditLogs, or change BE visibility/MoW paths. An extra prepared reanalysis is intentional for schema drift detection; no separate substantiated CPU, memory, metric, or logging issue was found. Every changed file and accepted/unresolved candidate was swept before submission.
User focus: review_focus.txt supplied no additional focus points.
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
|
/review |
|
Fixed the External Regression failure in 61a1c5d. The failing The latest four review findings are fixed in a781cf9. Validation passed: 48 focused FE tests, FE Checkstyle, a real local delayed-response Thrift test, and the expanded Flight RPC regression. The full external suite still needs validation on the updated CI build. The dedicated schema-only Flight suite is not included in the External Regression selection. |
There was a problem hiding this comment.
Static review complete for PR #68631 at 18bfadc. Request changes: one new P1 inline finding shows that prepared BE query execution discards its executor before validating the actual schema. The previously reported P2 for WARM UP SELECT with an empty source (r4134752120) still applies and is not repeated. The earlier P1 schema-analysis/SET race (r4131745680) is fixed on this head, so there are no confirmed existing P0/P1 IDs.
Critical checkpoints:
- Goal, scope, and tests: GetSchema and Prepare now derive schemas through scoped parsing, planning, and command metadata without running query fragments. FE unit tests and a Flight RPC regression cover ordinary queries, commands, types, invalid input, namespace changes, and session restoration. The changed remote-catalog Thrift timeout also matches its seconds-based caller and millisecond socket API. The tests do not exercise prepared BE execution through the real
FlightSqlConnectProcessor.close()path, which masks the new P1; the empty-source WARM UP case is also missing. - Concurrency and lifecycle: Schema analysis and Flight execution share the per-session monitor; the prior concurrent SET issue is addressed. Prepared handles reanalyze result-affecting settings and track namespace changes. The new post-execution comparison loses the executor during processor close, so valid BE field labels can expire the handle and mismatch cancellation cannot signal the BE. No separate lock-order, static-initialization, or ownership issue was substantiated.
- Parallel paths, conditions, and compatibility: I traced direct and prepared GetSchema/GetFlightInfo, FE-local and BE results, USE/SWITCH prefixes, no-row commands, and dynamic SHOW metadata against execution. The empty-source WARM UP fast path remains the already-commented exception. No new protocol symbol, storage format, or FE-to-BE session variable was added; Arrow wire-type and nested-field mappings were checked against the BE serializer. The P1 breaks prepared-result schema compatibility at runtime.
- Errors, resources, observability, and performance: Unsupported metadata paths return explicit errors rather than guessing headers; schema discovery avoids query execution and expensive PROC row fetches. Existing logs expose failures, but the P1's cancellation call is ineffective after executor cleanup. The removed prepared placeholder avoids its prior Arrow allocation. No transaction, persistence, storage-version, delete-bitmap, or data-write path is modified by schema discovery; no additional metric or hot-path regression was substantiated.
- Verification limits and focus: This was a static review only, as required; no build or test was run, so test results are not independently verified. There was no additional user-provided focus.
|
run buildall |
|
/review |
There was a problem hiding this comment.
Complete static review of PR #68631 at head f311672 (base 76d173e). I reviewed all 21 changed paths, the upstream and downstream execution paths, the existing inline threads, and the FE/BE Arrow schema contract. Two normal full-review subagents and a separate risk-focused subagent converged in one round with NO_NEW_VALUABLE_FINDINGS. There are no new inline findings.
The existing [P2] WARM UP empty-source thread (comment 4134752120) remains applicable: the early empty-insert return yields StatusResult while schema discovery advertises six statistics columns. I independently confirmed this path and did not repeat the comment. Historical [P1] comments 4131745680 and 4140024678 no longer apply: session execution now shares the schema-analysis monitor, and the final executor is retained before processor close for prepared schema comparison and cancellation. Accordingly, there are no still-applicable existing P0/P1 IDs.
Critical checkpoints:
- Goal and scope: the new schema-only path derives Flight headers without running the query; Prepare/GetSchema, prepared validation, command metadata, Arrow type conversion, and the remote Thrift timeout fix are localized to the affected components. The known WARM UP edge prevents complete execution/schema parity for that case.
- Concurrency and lifecycle: Flight session analysis and execution use the same context monitor; temporary session, statement, namespace and thread-local state is restored after analysis. Prepared execution captures its final executor before processor cleanup, compares the actual schema, then cancels and releases deferred resources on drift. The deferred executor list is detached under its own lock and finalized outside that lock. No new deadlock, state-loss, or cross-translation-unit initialization issue was substantiated. Planning and BE schema fetch can hold the per-session monitor, so concurrent RPCs for that session wait.
- Configuration and compatibility: no new configuration item, FE-BE transmitted variable, storage format, function symbol, or persistent record was introduced. FE Arrow type, nested child, nullability, timezone, and top-level logical metadata construction was checked against the BE Arrow serializer on this checkout. Rolling-upgrade behavior was not exercised.
- Parallel paths and conditions: direct and prepared schema calls, FE-local command results, BE query results, USE/SWITCH prefixes, no-row command categories, unsupported dynamic headers, and the empty-insert fast path were traced. The command-header exceptions avoid executing commands solely to discover metadata.
- Tests and results: the patch adds FE unit tests and a Flight RPC regression for positive and negative schema cases, session restoration, handle expiry, and drift cleanup. The new regression does not create schema_source/schema_target, so its DML and WARM UP cases prove metadata discovery rather than execution parity; the known empty-source case is not covered there. No builds or tests were run in this review, and no test outcome is claimed as independently verified. No result .out file was changed.
- Observability, persistence and writes: existing error propagation and logging cover schema-analysis and prepared-drift failures; no concrete missing metric was identified. Schema discovery does not start transaction or data writes and changes no EditLog/replay path. Direct command execution retains its existing write and transaction flow.
- Performance and other risks: prepared calls reanalyze SQL and session locking serializes same-session requests; this is visible in the implementation, but no separate correctness or material performance regression was substantiated. The FeServiceClient timeout now converts the caller's seconds to pooled-Thrift milliseconds for both affected RPC paths.
User focus: no additional focus was provided. Review execution is complete and static-only; the already-reported WARM UP P2 remains for the author to address.
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
|
/review |
|
Fixed the Flight failures from P0 build 1060727 in f44c66c. Schema-only analysis created QueryState with isQuery=false, so result-sink binding inferred command-style names such as __literal_0 and __unhex_0. Real StmtExecutor execution sets isQuery=true and keeps SQL labels such as 1 and unhex(encoded). Preparation now sets query state before analysis, preserving the execution labels without weakening schema-drift checks. Validated before committing against a real isolated local FE + BE cluster, using this PR's CI BE binary and the actual regression framework:
The separately muted mtmv_with_sql_cache failure also occurs on unrelated PRs and is outside this Flight fix. The local test cluster has been stopped. Full CI has been requested separately. |
|
run buildall |
|
/review |
|
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. |
There was a problem hiding this comment.
Complete static review of PR #68631 at d2c4a08 (all 23 changed files). One new P2 issue is inline: Prepare/GetSchema reject executable no-result SET and cloud USE prefixes before registering a prepared handle. The empty-source WARM UP result-schema mismatch still applies, but is already reported in r4134752120 and is not duplicated here. The earlier P1 session race (r4131745680) and lost-executor issue (r4140024678) are fixed on this head; no existing P0/P1 inline finding remains applicable. The review focus file specifies no additional points.
Critical checkpoints: The main goal—advertising analyzed Flight query/command schemas and table-type metadata—is implemented for the covered paths, subject to the new prefix regression and known WARM UP mismatch. FE schema analysis is isolated under the per-session monitor; normal and prepared execution use that monitor, the cloned session/query state is restored, and the retained final executor supports schema-drift cancellation and deferred cleanup. No independent lock-order, static-initialization, or lifecycle defect was substantiated. Command metadata was checked against FE-local result serialization, empty/conditional paths, and direct versus prepared execution; supported nested Arrow fields, scalar types, and table-type names were compared with the BE conversion and GetTables paths. The changed timeout uses existing seconds-based catalog configuration and the pooled Thrift socket's millisecond contract. No new configuration, FE-BE variable, persisted format, edit-log path, or transaction/write implementation was added, so those checkpoints are not directly exercised by this PR. Unsupported dynamic metadata fails explicitly; no separate observability or hot-path performance defect was substantiated. The changes are concentrated in FE schema/producer logic, command header helpers, the timeout fix, and supporting tests.
The added FE tests and Flight RPC regressions exercise representative query/command headers, types, session restoration, prepared lifecycle, table types, and negative cases. They do not cover the two executable multi-statement prefix examples or the already reported empty-source WARM UP path; metadata-only DML fixtures do not establish DML runtime behavior. This invocation ran no builds or tests, as required, so these are static code conclusions rather than independent runtime results. Three review rounds converged with no new valuable findings in the final full-review and risk passes, and the final changed-file/candidate sweep found no unresolved point.
| // JDBC clients commonly prefix their query with USE. Resolve that namespace only within this scope. | ||
| for (int i = 0; i < statements.size() - 1; ++i) { | ||
| Plan prefix = ((LogicalPlanAdapter) statements.get(i)).getLogicalPlan(); | ||
| if (!(prefix instanceof UseCommand) && !(prefix instanceof SwitchCommand)) { |
There was a problem hiding this comment.
[P2] Preserve preparation for executable no-result prefixes. SET query_timeout=17; SELECT 1 and, in cloud mode, USE @valid_group; SELECT 1 both execute with a single final Flight result: their first command sends no result set. Prepare now invokes this analyzer before registering a handle, so this guard rejects both requests with UNIMPLEMENTED although they were preparable before. Cloud USE is a UseCloudClusterCommand, not a UseCommand; its database-qualified form can also change the final schema's namespace. Resolve these prefixes against isolated session state, including settings and cloud namespace/group changes, or retain a compatible prepared path.
FE Regression Coverage ReportIncrement line coverage |
…ta (#68631) ### What problem does this PR solve? Flight SQL preparation currently returns the same `ResultMeta: string` schema for every SQL statement without analyzing it. Python ADBC `adbc_execute_schema()` consumes that prepared dataset schema directly, so valid queries report incorrect columns and invalid queries appear to succeed. Direct `GetSchema` is also unimplemented. Resolve output slots with the FE-only Nereids prepare planner and check table/column privileges before converting them to Arrow fields. Set query state before binding result columns, matching execution so unaliased expressions retain their SQL labels instead of synthetic command aliases. Both Prepare and GetSchema use this resolver, including nested types, nullability, decimal precision and timestamp units. Invalid queries fail before a prepared handle is registered; metadata requests do not construct a query executor or dispatch BE fragments. Preserve JDBC preparation of commands using their FE result metadata (including the actual `StatusResult` protocol schema for commands without rows), and resolve USE/SWITCH prefixes within a temporary namespace scope. Session hints, statement resources and worker thread context are restored on success and failure. Schema analysis and execution share a per-session lock, so concurrent execution cannot update temporary analysis variables. Dialect conversion and the configured original-SQL parse fallback run before analysis, and USE/SWITCH prefixes honor remembered catalog databases without changing session history. Parameter binding, arbitrary multi-statement batches, and commands without discoverable result metadata remain explicitly unsupported by schema discovery. Prepared handles are invalidated when used from a different catalog or database. After a successful prepared USE/SWITCH, that handle is rebound to its resulting namespace so it remains reusable; other handles retain their original namespace checks. Before GetSchema or prepared execution, reanalyze and compare against the schema advertised at Prepare; result-affecting session changes such as `sql_mode` require preparing again, while settings that preserve the schema keep the handle valid. Check the actual execution schema again before returning FlightInfo to cover concurrent DDL between analysis and replanning. For BE results, compare the final planner's semantic column labels and positional Arrow types: wire labels may contain expression text or a TVF prefix, and constant folding may narrow nullability. Keep nested field names, type metadata and dictionary encoding checks, and reject nullable widening. Retain the final executor alongside FlightInfo before processor cleanup clears the session context. On mismatch, expire the handle, cancel that retained executor before finalizing deferred executors, and clear buffered results/endpoints. Resolve HELP headers from the local help catalog and SHOW SNAPSHOT headers from validated predicates without reading snapshot data. Resolve fixed headers for SHOW CONFIG, SHOW CREATE TABLE, procedure SHOW commands, ordinary EXPLAIN, PLAN REPLAYER and WARM UP SELECT. Concrete no-row DML and transaction commands advertise `StatusResult`; Lance CREATE/DROP INDEX advertise `JobId` without admission or remote table resolution. SHOW PROC exposes static headers for `/` and `/current_queries` without opening nodes or fetching rows. Other PROC nodes, EXPLAIN PLAN PROCESS (which lacks Flight result serialization), DESCRIBE, SHOW DATA, SHOW PARTITIONS, SHOW QUERY STATS, SHOW PYTHON PACKAGES, COPY INTO and WARM UP CLUSTER remain explicitly unsupported by schema discovery. The string-serialized `group_concat_state` output maps to Arrow UTF8; unmodeled aggregate-state serializations remain unsupported. Prepare and prepared execution now incur an additional FE analysis pass to discover and validate the advertised schema. Physical planning and query execution remain in the existing execution path. The External Regression investigation also exposed an existing remote Doris metadata timeout unit mismatch: the catalog supplies seconds, but pooled Thrift sockets require milliseconds. Convert the configured value with overflow checking so the default 10 seconds does not become 10 milliseconds. Implement GetTableTypes data streaming for ADBC/JDBC metadata discovery. GetFlightInfo previously returned the standard schema and ticket, but DoGet always failed with UNIMPLEMENTED. Return sorted, distinct non-null public names from the existing TableType.toMysqlType mapping (BASE TABLE, SYSTEM VIEW, VIEW), using the standard non-null UTF8 field and releasing vectors on success or failure. This does not enumerate catalogs or execute queries; GetTables type-filter behavior is outside this change. ### Release note Fix Arrow Flight SQL schema discovery returning placeholder metadata and implement table-type metadata streaming for ADBC/JDBC clients. ### Check List (For Author) - Test - [x] Unit Test - [x] Regression Test - [x] Manual test - Behavior changed: - [x] Yes. Prepare validates queries and reports their real output schema; GetSchema is implemented without executing the query. - Does this need documentation? - [x] No. Validation: - FE Checkstyle passed. Recompiled changed FE classes and tests against the existing FE test build; all 62 focused tests passed. Execution-schema tests now run real FlightSqlConnectProcessor.handleQuery(), covering the query-state transition that direct planner tests missed. The updated tests failed for SELECT 1, unhex and CAST before the fix and passed afterward. Negative schema-drift and deferred-executor lifecycle tests remain covered. - Ran the actual regression framework against an isolated local FE + BE cluster using the BE binary from this PR's CI compile and current FE classes. With the previous FE classes, all five Flight suites from P0 build 1060727 reproduced schema-drift failures. With the fix, test_flight_timestamp_range, test_flight_utf8_validation, test_sql_cache_over_arrow_flight, test_select and test_auth_remote_ip all passed, along with the expanded test_schema_only and new test_table_types suites: 7 suites, 0 failures, 0 skipped. - GetTableTypes tests cover the full GetFlightInfo-to-DoGet RPC, standard schema, sorted unique values, repeated requests, JDBC metadata, subsequent same-connection queries, and vector cleanup on stream failure. Python ADBC also passed 10 consecutive adbc_get_table_types calls followed by a query against the local cluster. Both new FE tests failed before implementation and passed afterward. - The new persistent regression checks unaliased literal/function/CAST metadata and performs prepared execution, so metadata-only agreement cannot hide a mismatch with execution. - The separately muted mtmv_with_sql_cache failure also occurs on unrelated PRs and is not changed here. Full P0/external CI reruns remain separate from the focused local validation. - Earlier validation covered prepared namespace reuse, HELP and snapshot headers, session isolation, and the remote Doris timeout correction using a real local Thrift server with a delayed response. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
### What problem does this PR solve? Issue Number: None Related PR: apache#68631 Problem Summary: branch-4.1 apache#68631 (also on branch-4.2) implemented the GetTableTypes data stream and tested it end to end over Flight RPC: a unit test that runs a real FlightServer and FlightSqlClient, and a regression suite that uses a plain Flight SQL client. The tests in this PR call `getStreamTableTypes()` directly with a mocked listener, or go through JDBC `getTableTypes()`, so the GetFlightInfo -> DoGet sequence that a plain Flight SQL client sends was not covered. This commit adds both checks from apache#68631: - `FlightSqlTableTypesTest#tableTypesAreSortedAndStableThroughFlightRpc` calls GetTableTypes three times through a real FlightServer. Each call must return the GetTableTypes schema with one endpoint, and DoGet must return the same sorted types. Only DoGet resolves the session. The 4.x version mocked the ConnectContext and checked that nothing touched it. On master the metadata stream runs as a command of the session (`FlightProtocolAdapter.runCommand`), which reads the context, so the test uses `ConnectContext.forFlight` as the other tests here do. - `test_table_types_over_arrow_flight` sends the same RPCs three times from a plain Flight SQL client, checking the field name, type and nullability, then runs a query on the session afterwards. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test - Behavior changed: No - Does this need documentation: No Co-authored-by: Gabriel <liwenqiang@selectdb.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es, SHOW STORAGE POLICY and WARM UP SELECT ### What problem does this PR solve? Issue Number: None Related PR: apache#68631 Problem Summary: Review of this port found three statements that run over the Arrow Flight SQL JDBC driver on master but failed once Prepare started to analyze them. The driver prepares every statement and does not fall back when Prepare refuses one. 1. A CTE used once, under column-level privileges. Prepare checked privileges on the analyzed plan. There the CTE is still a `LogicalCTEAnchor` over its producer, and `ColumnPruning` treats every producer output as needed. So with only `SELECT_PRIV(id)` on `t(id, secret)`, `WITH c AS (SELECT id, secret FROM t) SELECT id FROM c` failed Prepare with INVALID_ARGUMENT. Execution runs the query, because it inlines the CTE first and then checks only the columns read. Prepare now runs `PullUpCteAnchor` and `CTEInline` on the analyzed plan before `CheckPrivileges`, as the rewriter does. The rewriter normalizes the plan further before its own check; for example, it drops an ORDER BY under a derived table. So a denial on the analyzed plan does not show that execution is denied. Such a statement now keeps the placeholder schema and runs unchecked, so execution decides as it did before this PR, and GetSchema answers UNIMPLEMENTED without a schema. 2. SHOW STORAGE POLICY. `ShowStoragePolicyCommand.getMetaData()` returned the four-column USING header for every form, but the plain form returns the seven columns of `StoragePolicy.STORAGE_META_DATA`. Every prepared execution therefore failed with NOT_FOUND "Prepared statement schema changed". It now returns the header that `doRun` uses for each form. This also corrects the column definitions that MySQL `COM_STMT_PREPARE` reports for the statement. 3. WARM UP SELECT. When planning proves the source empty (a predicate that folds to false, no partition left after pruning, or partitions without data), the statement returns OK instead of its statistics, and its prepared execution failed as a schema change. Its header depends on what planning finds, so WARM UP SELECT now keeps the placeholder schema like the other statements whose result depends on execution. The `getResultSetMetaData()` override added for it is removed. The regression suite `test_schema_only` now also executes its side-effect-free frontend-side statements as prepared statements, compares the executed schema with the advertised one, and runs a WARM UP SELECT on an empty table. ### Release note Flight SQL Prepare keeps the placeholder schema for WARM UP SELECT and for queries whose privilege check fails; those queries still fail when they run. The prepared metadata of SHOW STORAGE POLICY lists the seven columns it returns. ### Check List (For Author) - Test: Unit Test, Regression test - `DorisFlightSqlSchemaTest`: new `columnGrantsAreCheckedAsExecutionChecksThem` and `warmupSelectRunsUncheckedWhateverItsSourceHolds`; updated `schemaDiscoveryChecksTablePrivileges` and `commandsWithUnknownOrDynamicMetadataKeepThePlaceholder` - `SchemaOnlyCommandMetadataTest`: new `storagePolicyHeadersMatchExecution` - `arrow_flight_sql_p0/test_schema_only`: prepared executions of frontend-side commands, and WARM UP SELECT on an empty table (its execution only in cloud mode, the only mode that warms up internal tables) - 41 FE test classes (536 tests) pass and fe-core Checkstyle is clean; skipping the inlining, failing Prepare on a denial again, returning the USING header for every form and validating WARM UP SELECT each turn their tests red - Local cluster: all 21 suites under `arrow_flight_sql_p0` and `test_uuid_arrow_flight` pass; the strengthened `test_schema_only` fails on the previous head at the SHOW STORAGE POLICY header - Behavior changed: Yes. A query whose privilege check on the analyzed plan fails now keeps the placeholder at Prepare and fails at execution. WARM UP SELECT keeps the placeholder. SHOW STORAGE POLICY reports its seven columns. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… schema ### What problem does this PR solve? Issue Number: None Related PR: apache#68631 Problem Summary: Prepare advertised an untyped NULL result column (NULL_TYPE) as Arrow `Null`, the type `DorisArrowTypeMapping` gives NULL_TYPE. BE never sends that: `data_type_factory.cpp` builds the column as a `DataTypeUInt8` flagged as a NULL literal, whose primitive type is BOOLEAN, so `convert_to_arrow_type` sends nullable `Bool`. The comparison after a prepared execution therefore failed, removed the handle and answered NOT_FOUND "Prepared statement schema changed; prepare again" for every execution of such a query. Over the Arrow Flight SQL JDBC driver, which prepares every statement, `select org_id, null from dim_org ...` in `query_p0/operator/test_sort_operator` failed this way. Before apache#69867 the query ran, because Prepare always returned the placeholder schema. `FlightSqlQuerySchema.arrowField` already treats a result type that maps to Arrow `Null` as one this frontend cannot describe as BE sends it, and keeps the placeholder for it, except for NULL_TYPE. It now covers NULL_TYPE too, at any depth since `arrowField` recurses into ARRAY, MAP and STRUCT children: Prepare returns the placeholder, the executions are not checked, and GetSchema answers UNIMPLEMENTED, as on master before apache#69867. A NULL with a type (`CAST(NULL AS VARCHAR(20))`) is still described as that type. Advertising `Bool` instead would tie the frontend to how BE happens to store a NULL literal. ### Release note Flight SQL Prepare keeps the placeholder schema for a result with an untyped NULL column, so its prepared executions run instead of failing with "Prepared statement schema changed". ### Check List (For Author) - Test: Unit Test, Regression test - `DorisFlightSqlSchemaTest#untypedNullResultsKeepThePlaceholder`: `SELECT NULL`, `SELECT id, NULL ... ORDER BY id`, and NULL inside ARRAY, MAP and STRUCT keep the placeholder, their handles carry no schema to check, and GetSchema answers UNIMPLEMENTED. It fails with the previous condition restored. - `arrow_flight_sql_p0/test_schema_only` adds `SELECT NULL` and `SELECT ARRAY(NULL)` to the placeholder statements and runs a prepared `SELECT k, NULL AS n ...` twice through the backends. - 41 FE test classes (537 tests) pass and fe-core Checkstyle is clean. - Local cluster: `test_sort_operator` over Flight SQL fails with "Prepared statement schema changed" on the previous head and passes on this one; all 20 suites under `arrow_flight_sql_p0` and `test_uuid_arrow_flight` pass; the strengthened `test_schema_only` fails on the previous head at `SELECT NULL AS n`. - Behavior changed: Yes. A result with an untyped NULL column keeps the placeholder schema on Prepare, as master does without this port. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…red handle ### What problem does this PR solve? Issue Number: None Related PR: apache#68631 Problem Summary: A prepared GetFlightInfo checks its handle before it executes anything: the handle must exist, the session must still be in the namespace it was prepared in, and the statement re-analyzed now must have the schema that Prepare advertised. `executePreparedStatement` ran these checks before `executeQueryStatement` reached `FlightProtocolAdapter.beginRequest()`, where a request drops what the previous one left: its deferred coordinator, the frontend-side result no DoGet took, and its endpoints. A handle that the checks refused therefore left all three in place. The previous query's deferred coordinator, which a batch-mode external-table scan or a remote Doris scan leaves, kept its workload group queue slot and its query registration until the session's next statement, the timeout checker or teardown, and the previous frontend-side result could still be fetched. Before apache#69867, every prepared GetFlightInfo reached `executeQueryStatement`, and a missing handle failed inside it, where the catch finalized the previous coordinator. `executePreparedStatement` now calls `beginRequest()` before the checks: a prepared execution is the session's next request whatever they find. Prepare and GetSchema are metadata requests and still leave the previous request alone, so the call is neither in the session's command lock nor in `preparedQuery()`, which GetSchema goes through too. A checked handle whose statement could no longer be described when it was re-analyzed answered UNIMPLEMENTED and was kept, so every later execution failed the same way. That happens after a column grant is revoked so that the analyzed plan is denied, or after a DDL that turns a result column into an untyped NULL. The check now compares the schema Prepare would advertise now, the placeholder for such a statement: the handle is removed and the request answers NOT_FOUND "Prepared statement schema changed; prepare again", as for any other schema change, and the statement prepared again keeps the placeholder and runs unchecked. Prepare and the check share `advertisedSchema` for this. ### Release note A Flight SQL prepared execution refused before it runs no longer keeps the previous query's deferred coordinator and result. A prepared statement whose result schema can no longer be determined before it runs answers NOT_FOUND and must be prepared again. ### Check List (For Author) - Test: Unit Test, Regression test - `DorisFlightSqlSchemaTest` adds one test for each way the checks refuse a handle: a missing handle, a namespace change, a sql_mode drift, a dropped view (INVALID_ARGUMENT, the handle is kept) and a revoked column grant (NOT_FOUND, the handle is removed, and preparing again gives the placeholder). Each sends GetSchema and GetFlightInfo after a previous request left a deferred executor, a frontend-side result and an endpoint, and checks that only GetFlightInfo finished it. - Without the new `beginRequest()` call the five tests fail at GetFlightInfo; with the call in `preparedQuery()` they fail at GetSchema; with the previous UNIMPLEMENTED only the revoked-grant test fails. - 45 FE test classes (555 tests) pass and fe-core Checkstyle is clean. - Local cluster: all 21 suites under `arrow_flight_sql_p0`, `test_uuid_arrow_flight` and `test_sort_operator` over Flight SQL pass. - Behavior changed: Yes. A prepared GetFlightInfo that its checks refuse finishes the previous request first. A checked handle that can no longer be described is removed and answers NOT_FOUND instead of UNIMPLEMENTED. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…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 preparation currently returns the same
ResultMeta: stringschema for every SQL statement without analyzing it. Python ADBCadbc_execute_schema()consumes that prepared dataset schema directly, so valid queries report incorrect columns and invalid queries appear to succeed. DirectGetSchemais also unimplemented.Resolve output slots with the FE-only Nereids prepare planner and check table/column privileges before converting them to Arrow fields. Set query state before binding result columns, matching execution so unaliased expressions retain their SQL labels instead of synthetic command aliases. Both Prepare and GetSchema use this resolver, including nested types, nullability, decimal precision and timestamp units. Invalid queries fail before a prepared handle is registered; metadata requests do not construct a query executor or dispatch BE fragments.
Preserve JDBC preparation of commands using their FE result metadata (including the actual
StatusResultprotocol schema for commands without rows), and resolve USE/SWITCH prefixes within a temporary namespace scope. Session hints, statement resources and worker thread context are restored on success and failure. Schema analysis and execution share a per-session lock, so concurrent execution cannot update temporary analysis variables. Dialect conversion and the configured original-SQL parse fallback run before analysis, and USE/SWITCH prefixes honor remembered catalog databases without changing session history. Parameter binding, arbitrary multi-statement batches, and commands without discoverable result metadata remain explicitly unsupported by schema discovery.Prepared handles are invalidated when used from a different catalog or database. After a successful prepared USE/SWITCH, that handle is rebound to its resulting namespace so it remains reusable; other handles retain their original namespace checks. Before GetSchema or prepared execution, reanalyze and compare against the schema advertised at Prepare; result-affecting session changes such as
sql_moderequire preparing again, while settings that preserve the schema keep the handle valid. Check the actual execution schema again before returning FlightInfo to cover concurrent DDL between analysis and replanning. For BE results, compare the final planner's semantic column labels and positional Arrow types: wire labels may contain expression text or a TVF prefix, and constant folding may narrow nullability. Keep nested field names, type metadata and dictionary encoding checks, and reject nullable widening. Retain the final executor alongside FlightInfo before processor cleanup clears the session context. On mismatch, expire the handle, cancel that retained executor before finalizing deferred executors, and clear buffered results/endpoints.Resolve HELP headers from the local help catalog and SHOW SNAPSHOT headers from validated predicates without reading snapshot data. Resolve fixed headers for SHOW CONFIG, SHOW CREATE TABLE, procedure SHOW commands, ordinary EXPLAIN, PLAN REPLAYER and WARM UP SELECT. Concrete no-row DML and transaction commands advertise
StatusResult; Lance CREATE/DROP INDEX advertiseJobIdwithout admission or remote table resolution. SHOW PROC exposes static headers for/and/current_querieswithout opening nodes or fetching rows. Other PROC nodes, EXPLAIN PLAN PROCESS (which lacks Flight result serialization), DESCRIBE, SHOW DATA, SHOW PARTITIONS, SHOW QUERY STATS, SHOW PYTHON PACKAGES, COPY INTO and WARM UP CLUSTER remain explicitly unsupported by schema discovery. The string-serializedgroup_concat_stateoutput maps to Arrow UTF8; unmodeled aggregate-state serializations remain unsupported.Prepare and prepared execution now incur an additional FE analysis pass to discover and validate the advertised schema. Physical planning and query execution remain in the existing execution path.
The External Regression investigation also exposed an existing remote Doris metadata timeout unit mismatch: the catalog supplies seconds, but pooled Thrift sockets require milliseconds. Convert the configured value with overflow checking so the default 10 seconds does not become 10 milliseconds.
Implement GetTableTypes data streaming for ADBC/JDBC metadata discovery. GetFlightInfo previously returned the standard schema and ticket, but DoGet always failed with UNIMPLEMENTED. Return sorted, distinct non-null public names from the existing TableType.toMysqlType mapping (BASE TABLE, SYSTEM VIEW, VIEW), using the standard non-null UTF8 field and releasing vectors on success or failure. This does not enumerate catalogs or execute queries; GetTables type-filter behavior is outside this change.
Release note
Fix Arrow Flight SQL schema discovery returning placeholder metadata and implement table-type metadata streaming for ADBC/JDBC clients.
Check List (For Author)
Validation:
Check List (For Reviewer who merge this PR)