refactor: drive datafusion.runtime.* config from a ConfigField schema - #25114
Draft
haochunchang wants to merge 2 commits into
Draft
refactor: drive datafusion.runtime.* config from a ConfigField schema#25114haochunchang wants to merge 2 commits into
datafusion.runtime.* config from a ConfigField schema#25114haochunchang wants to merge 2 commits into
Conversation
haochunchang
marked this pull request as draft
September 9, 2026 13:42
…hema The eight `datafusion.runtime.*` keys were spelled out 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 apache#23316. Add `datafusion_execution::runtime_options::RuntimeOptions`, built with the `config_namespace!` / `config_field!` macros that already back `ConfigOptions`, and make it the single source of the keys, descriptions, defaults and parsing. `SET` and `RESET` now dispatch through `ConfigField`, so adding a runtime option is a one-line struct field. `RuntimeEnvBuilder::entries` reads its defaults from the same constants the builder uses instead of hardcoded strings such as `"100G"`, which removes a silent drift risk. `RuntimeOptions` deliberately stays outside `ConfigOptions`: `datafusion-execution` depends on `datafusion-common`, never the reverse. It is also not a `ConfigExtension`, because extension prefixes may never be `datafusion`. Three per-key listings remain and are now all in one file: the schema, the `from_runtime_env` read of the live resource objects, and the `apply_key` write onto `RuntimeEnvBuilder`. The last two cannot merge into the schema because the builder setters have per-field signatures, and because reported values must come from the live objects rather than from whatever was last requested. Public API is unchanged. `SessionContext::parse_capacity_limit` keeps its signature, its doctest and its behaviour, and now delegates to the shared parser. `RuntimeEnv` and `RuntimeEnvBuilder` keep every public field and setter. Error text changes. `ConfigField::set` is not given the key it is setting, so a leaf parser cannot 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. Four assertions are updated to match. `RuntimeEnv::config_entries` output is unchanged, including `unlimited` for an unbounded pool, and the generated `configs.md` runtime table is byte-identical apart from prettier's column padding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TpQVFSJ6v2yfMNqwbsfsRe
Three cuts, no behaviour change: - `SET`/`RESET` built a `RuntimeOptions` from the live `RuntimeEnv` before writing one key into it. Since only that one key is ever applied, the read was wasted; both paths now start from `RuntimeOptions::default()`. - `RuntimeEnv::config_entries` knew the namespace prefix and the `unlimited` special case. That moves into `RuntimeOptions::env_entries`, so all key knowledge now lives in `runtime_options.rs` and `config_entries` is a one-line delegate. `from_runtime_env` becomes private. - Trimmed the module and type docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TpQVFSJ6v2yfMNqwbsfsRe
haochunchang
force-pushed
the
refactor/23322-runtime-options-config-field
branch
from
September 9, 2026 15:12
5151f62 to
eb150c4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
SessionConfigandRuntimeConfiginto same framework #23322.Rationale for this change
The eight
datafusion.runtime.*keys were spelled out by hand in six places: theprivate
RuntimeConfigValuesstruct and itsinto_config_entries, the valuederivation in
RuntimeEnv::config_entries, a hardcoded default table inRuntimeEnvBuilder::entries, and a hand-writtenmatcharm per key in bothSessionContext::set_runtime_variableandreset_runtime_variable. A comment inset_runtime_variableasked contributors to remember to updatereset_runtime_variablewhen adding an option, and the hand-rolled parse arms arethe code that produced the panic fixed in #23316.
#23322 asks whether runtime config can use the same type-checked framework as
SessionConfig/ConfigOptionsinstead of its own hand-rolledSET/RESETpath.This PR gets there for the parsing/dispatch framework:
RuntimeOptionsis builtwith the same
config_namespace!/config_field!macros that backConfigOptions, soSET/RESETnow dispatch throughConfigFieldand adding aruntime option is a one-line struct field instead of edits in six places.
RuntimeOptionsdeliberately stays a separate struct rather than fields onConfigOptionsitself:datafusion-executiondepends ondatafusion-common,never the reverse, so
ConfigOptions(which lives indatafusion-common) cannothold it. It is also not a
ConfigExtension, because extension prefixes may neverbe
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
SessionConfigs and can change mid-execution); this PR onlyunifies the framework they're built on.
What changes are included in this PR?
datafusion_execution::runtime_options::RuntimeOptions, the single sourceof the eight
datafusion.runtime.*keys, their descriptions, defaults andparsing.
SessionContext::set_runtime_variable/reset_runtime_variablenow build aRuntimeOptions::default()and dispatch throughConfigField::set, instead ofa hand-written
matchper key.parse_capacity_limitkeeps its signature,doctest and behavior, delegating to the shared parser.
RuntimeEnvBuilder::entriesreads its defaults from the same constants thebuilder uses instead of hardcoded strings such as
"100G", removing a silentdrift risk between the builder's actual defaults and the table shown to users.
RuntimeEnv::config_entriesis now a one-line delegate toRuntimeOptions::env_entries, which owns thedatafusion.runtimenamespaceprefix and the
unlimited-pool special case;from_runtime_envis private.from_runtime_envread of thelive resource objects, and the
apply_keywrite ontoRuntimeEnvBuilder) andare now all in one file,
runtime_options.rs. The last two can't merge intothe 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::setisn't given the key it'ssetting, 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 andwhen setting '<key>'in others.Public API is unchanged:
RuntimeEnvandRuntimeEnvBuilderkeep every publicfield and setter.
What is the testing strategy for this PR?
datafusion/execution/src/runtime_options.rsunit tests (parsing, defaults,SET/RESETround-trips for every key) —cargo test -p datafusion-execution --lib runtime_options.datafusion/core/tests/sql/runtime_config.rsintegration tests,updated for the new
when setting '<key>'error wording —cargo test -p datafusion --test core_integration runtime_config.datafusion/sqllogictest/test_files/set_variable.sltcases, updatedfor the same error wording.
SessionContext::parse_capacity_limitdoctest.docs/source/user-guide/configs.mdis unchanged: ran./dev/update_config_docs.shand diffed — the generated runtime table isbyte-identical, so it isn't touched by this PR.
cargo fmt --all -- --checkandcargo clippy --all-targets --all-features(
datafusion-executionanddatafusion) 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.
🤖 Generated with Claude Code