morningman commented on PR #67966:
URL: https://github.com/apache/doris/pull/67966#issuecomment-5677456650
Addressed in 674e491f9a9 (review target was ad7c70ba0c5). Per finding, each
answered in its thread:
- **[P1] Prevent teardown from racing deferred publication** — fixed:
teardown tombstones the adapter (`FlightProtocolAdapter.tearDown`, from
`unregisterConnection`); what a still-running command defers afterwards is
finalized on the spot by that command, and a command that gets its turn after
teardown is refused with `UNAUTHENTICATED`. The window was the whole run of the
command before `deferForArrowFlight()`, not the publication instant, and exists
on master; the tombstone covers all of it. Latch-based test through the real
KILL path.
- **[P2] Preserve session-option scalar types in GET** — not changed. Every
ADBC getter is exact on the variant, the string one included, so native types
would break `SetOption("77")` → `GetOption` the same way strings break
`SetOptionInt` → `GetOptionInt`; text is the one representation `SHOW
VARIABLES` shows and `SET` accepts back, as documented in the class javadoc and
above ("read a variable with the string getter").
- **[P1] Freeze every delayed executor's query ID** — the two-backend-result
request is now refused in `finishStatement` before a second query id exists
(see the next-but-one item), and the remaining fallback to `context.queryId()`
is what `finalizeQuery` has always done for an executor finalized within its
own statement, on every protocol. Capturing a registration id per executor is a
`StmtExecutor` refactor beyond this PR.
- **[P2] Enable `Profile` from the same effective decision** — not this PR:
`Profile` is constructed from `enableProfile()` at construction time in both
`StmtExecutor` constructors, on every protocol; master loses the same profile
through the same early return in `Profile.updateSummary`. The freeze neither
causes nor cures it (it fixes the deferred-query profile that was published as
RUNNING and never finished).
- **[P2] Reject schema clears inside active transactions** — fixed: the
empty value and the empty string are refused with `ERROR` inside a transaction,
like the `USE` / `SWITCH` / `SET` the other options run, nothing mutated. Real
`BEGIN` / `ROLLBACK` in `FlightSessionOptionsTest`.
- **[P1] Unregister failed replan attempts under their own IDs** — not this
PR: the retry path's finally (`if (context.isReturnResultFromLocal())
finalizeQuery()`) and the "result on the backends before `registerQuery`"
ordering are the same on apache/master, so a replan-retried attempt leaves its
registration behind there exactly as here. A fix to the retry path (finalize an
attempt that failed with its result on the backends) will be a separate PR.
- **[P1] Enforce one result for backend-result statements** — fixed:
`finishStatement` counts a result left on the backends like one cached on the
frontend, so `SELECT; SELECT` and `SELECT; SET` are refused with
`ERR_ARROW_FLIGHT_SQL_MUST_ONLY_RESULT_STMT` before the next statement runs
(the SET does not run); a failed statement keeps its own error. The check had
counted frontend-side results only since before #67900.
Locally: FE UT green (`FlightProtocolAdapterTest` 13,
`FlightSessionOptionsTest` 15, `FlightSqlConnectPoolMgrTest` 2,
`FlightSqlDeferredQueryIdleTimeoutTest` 9, `DorisFlightSqlProducerTest` 7,
`StmtExecutorTest` 25, `ConnectContextTest` 31, `MysqlProtocolAdapterTest` 11),
fe-core checkstyle 0 violations, `arrow_flight_sql_p0` 10 suites green (the
docker suite skipped as always) against an FE built from 674e491f9a9, the two
new multi-statement refusals visible in fe.log.
--
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]
