morningman opened a new pull request, #69867:
URL: https://github.com/apache/doris/pull/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 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 or a missing privilege | fails only at execution | fails at
Prepare with INVALID_ARGUMENT |
| 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 and checks privileges, as 4.x does: 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.
- 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, WARM UP
SELECT, and EXPLAIN PLAN PROCESS (Rule / Before / After).
- 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. On a mismatch, the handle is removed, the execution is cancelled, and
the deferred coordinator, the channel result and the endpoints are dropped. 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.
- 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, including a `group_concat_state` AGG_STATE, which BE sends as
Utf8.
### Release note
Flight SQL `CreatePreparedStatement` and `GetSchema` return the analyzed
result schema of a statement. Statements whose schema is only known once they
run keep returning the placeholder schema. A prepared statement is refused with
NOT_FOUND and must be prepared again when the session's catalog or database, or
the result schema, changed since it was prepared. Invalid SQL and missing
privileges are reported at Prepare. Flight SQL now describes TIMEV2 columns as
Float64 and VARBINARY columns as Binary, matching the data it sends.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- New `arrow_flight_sql_p0/test_schema_only`. On a local cluster
built from this branch, all 21 suites under `arrow_flight_sql_p0` and
`datatype_p0/uuid/test_uuid_arrow_flight` pass.
`external_table_p0/iceberg/test_iceberg_arrow_flight_split_source` needs the
docker environment and was not run locally.
- [x] Unit Test
- New `DorisFlightSqlSchemaTest`, `FlightSqlExecutionSchemaTest`,
`SchemaOnlyCommandMetadataTest` and `FeServiceClientTest`. Updated
`DorisArrowTypeMappingTest` and `DorisFlightSqlProducerTest`; its allocator
leak test now uses statements that keep the placeholder. Together with the
existing tests of the changed classes (`mvn -pl :fe-core -am test`, Checkstyle
included): 32 classes, 390 tests, all pass.
- Each of three mutations makes only its own tests fail: skipping
the comparison after execution, answering UNIMPLEMENTED at Prepare as
branch-4.x does, and dropping the caller-prefix check on handles.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. Prepare returns the analyzed schema, and GetSchema is
implemented. Invalid SQL and missing privileges fail at Prepare. A prepared
handle is refused after a namespace or schema change, or when another session
presents it. TIMEV2 and VARBINARY are described as Float64 and Binary in
GetTables and Prepare schemas.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
