haochunchang opened a new pull request, #25114:
URL: https://github.com/apache/datafusion/pull/25114
## Which issue does this PR close?
- Closes #23322.
## Rationale for this change
The eight `datafusion.runtime.*` keys were spelled out by hand in six
places: the
private `RuntimeConfigValues` struct and its `into_config_entries`, the value
derivation in `RuntimeEnv::config_entries`, a hardcoded default table in
`RuntimeEnvBuilder::entries`, and a hand-written `match` arm per key in both
`SessionContext::set_runtime_variable` and `reset_runtime_variable`. A
comment in
`set_runtime_variable` asked contributors to remember to update
`reset_runtime_variable` when adding an option, and the hand-rolled parse
arms are
the code that produced the panic fixed in #23316.
#23322 asks whether runtime config can use the same type-checked framework as
`SessionConfig`/`ConfigOptions` instead of its own hand-rolled `SET`/`RESET`
path.
This PR gets there for the parsing/dispatch framework: `RuntimeOptions` is
built
with the same `config_namespace!` / `config_field!` macros that back
`ConfigOptions`, so `SET`/`RESET` now dispatch through `ConfigField` and
adding a
runtime option is a one-line struct field instead of edits in six places.
`RuntimeOptions` deliberately stays a separate struct rather than fields on
`ConfigOptions` itself: `datafusion-execution` depends on
`datafusion-common`,
never the reverse, so `ConfigOptions` (which lives in `datafusion-common`)
cannot
hold it. It is also not a `ConfigExtension`, because extension prefixes may
never
be `datafusion`. Fully merging the two option sets is the "independent
project"
the issue itself calls out as a separate, harder problem (runtime config is
shared across `SessionConfig`s and can change mid-execution); this PR only
unifies the framework they're built on.
## What changes are included in this PR?
- Add `datafusion_execution::runtime_options::RuntimeOptions`, the single
source
of the eight `datafusion.runtime.*` keys, their descriptions, defaults and
parsing.
- `SessionContext::set_runtime_variable` / `reset_runtime_variable` now
build a
`RuntimeOptions::default()` and dispatch through `ConfigField::set`,
instead of
a hand-written `match` per key. `parse_capacity_limit` keeps its signature,
doctest and behavior, delegating to the shared parser.
- `RuntimeEnvBuilder::entries` reads its defaults from the same constants the
builder uses instead of hardcoded strings such as `"100G"`, removing a
silent
drift risk between the builder's actual defaults and the table shown to
users.
- `RuntimeEnv::config_entries` is now a one-line delegate to
`RuntimeOptions::env_entries`, which owns the `datafusion.runtime`
namespace
prefix and the `unlimited`-pool special case; `from_runtime_env` is
private.
- Three per-key listings remain (the schema, the `from_runtime_env` read of
the
live resource objects, and the `apply_key` write onto `RuntimeEnvBuilder`)
and
are now all in one file, `runtime_options.rs`. The last two can't merge
into
the schema: the builder setters have per-field signatures, and reported
values
must come from the live resource objects rather than from whatever was last
requested.
Error text changes as a side effect: `ConfigField::set` isn't given the key
it's
setting, so a leaf parser can't name the key mid-message the way the old
hand-written arms did. Errors now carry the key as a `when setting '<key>'`
suffix, which also makes the wording consistent — the previous messages used
`for '<key>'` in some arms and `when setting '<key>'` in others.
Public API is unchanged: `RuntimeEnv` and `RuntimeEnvBuilder` keep every
public
field and setter.
## What is the testing strategy for this PR?
- `datafusion/execution/src/runtime_options.rs` unit tests (parsing,
defaults,
`SET`/`RESET` round-trips for every key) — `cargo test -p
datafusion-execution
--lib runtime_options`.
- Existing `datafusion/core/tests/sql/runtime_config.rs` integration tests,
updated for the new `when setting '<key>'` error wording — `cargo test -p
datafusion --test core_integration runtime_config`.
- Existing `datafusion/sqllogictest/test_files/set_variable.slt` cases,
updated
for the same error wording.
- `SessionContext::parse_capacity_limit` doctest.
- Confirmed `docs/source/user-guide/configs.md` is unchanged: ran
`./dev/update_config_docs.sh` and diffed — the generated runtime table is
byte-identical, so it isn't touched by this PR.
- `cargo fmt --all -- --check` and `cargo clippy --all-targets
--all-features`
(`datafusion-execution` and `datafusion`) are clean.
## Are there any user-facing changes?
Only error-message wording for invalid `SET`/`RESET datafusion.runtime.*`
values
(now uses a consistent `when setting '<key>'` suffix — see above). No
behavior,
default, or public API change.
--
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]
