Skip to content

Support '0' value for parse_capacity_limit()#22014

Open
mkleen wants to merge 1 commit intoapache:mainfrom
mkleen:capacity-0
Open

Support '0' value for parse_capacity_limit()#22014
mkleen wants to merge 1 commit intoapache:mainfrom
mkleen:capacity-0

Conversation

@mkleen
Copy link
Copy Markdown
Contributor

@mkleen mkleen commented May 5, 2026

Which issue does this PR close?

  • None

Rationale for this change

This extends parse_capacity_limit() to support `0` to set a limit of 0 instead of using `0K`.
This simplifies configuration and avoids confusion with the word `OK` (Okay).
This is based on the suggestion from @martin-g.

Usage:

SET datafusion.runtime.example_limit = '0'

instead of:

SET datafusion.runtime.example_limit = '0K'

What changes are included in this PR?

see above.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Copy link
Copy Markdown
Contributor

@kosiew kosiew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkleen
The change looks good overall and I did not find any blocking issues. I left one small suggestion around SQL-level regression coverage to help ensure the config path stays wired correctly.

"Empty limit value found for '{config_name}'"
));
}
if limit == "0" {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition to cover the helper directly 👍

Since the intended user-facing flow is SET datafusion.runtime.* = '0', it could also be helpful to add a small SQL-level regression test or SLT that exercises something like SET datafusion.runtime.memory_limit = '0' and verifies SHOW returns the expected value.

That would help catch any future wiring regressions between the parser and the config layer.

@kosiew
Copy link
Copy Markdown
Contributor

kosiew commented May 6, 2026

@mkleen

datafusion/execution/src/runtime_env.rs
in fn create_runtime_config_entries, the runtime config descriptions shown by information_schema/SHOW still say capacity limits support only K/M/G suffixes.

Since this PR adds bare 0 as a supported SQL-facing value for all parse_capacity_limit callers, consider updating the descriptions for memory_limit, max_temp_directory_size, metadata_cache_limit, and list_files_cache_limit to mention the special 0 value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants