andygrove opened a new pull request, #2416:
URL: https://github.com/apache/datafusion-ballista/pull/2416
> **Disclaimer:** this PR was implemented with an LLM (Claude Code) at my
request.
> It is **WIP / experimental**, opened for design discussion on #2298 rather
than
> as a merge candidate. Please read "Status" below before reviewing in depth
—
> the end-to-end tests have not been run.
# Which issue does this PR close?
Part of #2298. Does not close it.
# Rationale for this change
Ballista has no way for a generic SQL client to connect. Flight SQL was
removed
in 46.0.0 (#1227, #1228) because it was unmaintained, substantially
incomplete,
structurally coupled to `SchedulerServer`, and had zero tests. This is a
fresh
implementation aimed at those four objections, not a revival of the old
`flight_sql.rs`.
The concrete motivation was querying a Ballista cluster from Python via
`adbc_driver_flightsql`, bypassing the Ballista Python bindings.
# What changes are included in this PR?
**New crate `ballista/flight-sql`**, depending only on `ballista-core` — it
never references `SchedulerServer`.
- `backend.rs` — a `QueryBackend` trait (session open/close,
submit-and-await,
cancel), the transport-neutral seam #2298 asked for.
- `service.rs` — `BallistaFlightSqlService<B: QueryBackend>`: handshake,
`CommandStatementQuery`, `DoGet`, prepared statements,
`CommandStatementUpdate`,
`CancelQuery`.
- `metadata.rs` — `SqlInfo`, `XdbcTypeInfo`, and catalog introspection
sourced
from the session's real DataFusion catalog.
- `auth.rs` — pluggable `Authenticator`, no built-in credentials.
- `session.rs` — TTL'd stores for sessions, prepared statements, and results.
**Scheduler wiring** — `ballista/scheduler/src/flight_sql.rs` (~100 lines)
implements `QueryBackend`, behind a non-default `flight-sql` feature and a
`--flight-sql` runtime flag.
**Supporting changes** — `flight_proxy_service.rs` moves from
`ballista/scheduler/src` to `ballista/core/src` unchanged so both crates can
use
it; `start_grpc_service_with_listener` extracted so callers can bind port 0;
`docs/source/user-guide/flightsql.md` replaces the page deleted in #1228;
`examples/python/adbc_flight_sql.py`; a CI step running the tests with the
feature on.
# Design decisions worth discussing
Detail and rationale for each is in #2298; summarising what this PR chose:
1. **Endpoints carry no location**, so clients redeem tickets on the
connection
they already have and no executor address leaks. Direct fix for
#1012/#1349.
2. **The frontend subsumes the plain proxy** (only one can serve
`FlightService`); `do_get_fallback` keeps Ballista's own tickets working.
3. **Catalog model: option (b)** — embedder-supplied via `SessionBuilder`,
plus
session-scoped DDL.
4. **`advertise_flight_sql_endpoint` left alone** (#2298 item 8); a separate
`--flight-sql` flag was added instead.
5. **CTAS, `INSERT`/`UPDATE`/`DELETE`/`COPY` are refused** rather than
silently
executed on the scheduler.
6. **`CleanJobData` not wired** — the scheduler's existing delayed cleanup
owns
result retention.
Not implemented: `PollFlightInfo` (so `GetFlightInfo` blocks until the query
finishes), bound parameters, transactions, Substrait, key metadata.
**Security — the main reason this is marked experimental.** The default
authenticator accepts every handshake. Separately, the Flight proxy trusts
the
executor address carried in a ticket, so a forged ticket can make the
scheduler
dial an arbitrary host and relay the response. That second one is
pre-existing
in `BallistaFlightProxyService`, but enabling Flight SQL is what makes the
port
something you would consider exposing. Both are documented; neither is fixed
here, and the fix wants its own issue.
# Status
**Please do not read green CI as end-to-end validation.**
Passing locally: 28 tests — 16 unit, plus 12 protocol-level tests driving the
`FlightSqlService` trait against a stub backend and asserting on decoded wire
bytes rather than crate internals. The scheduler's 350 unit tests pass with
the
feature enabled; clippy, rustfmt, `--locked` and rustdoc are clean.
**Not run:** the three end-to-end tests in
`ballista/scheduler/tests/flight_sql.rs` (real scheduler + executor, driven
by
`FlightSqlServiceClient`). They compile, but the environment this was
developed
in blocks TCP `bind`, so the live scheduler↔executor↔client path is
unverified,
as is the Python ADBC example.
# Are there any user-facing changes?
Only opt-in. `flight-sql` is a non-default compile-time feature and
`--flight-sql` is off at runtime, so no existing deployment changes
behaviour.
Adds a user guide page and a Python example.
--
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]
