feat: add RESET statement for configuration variabless #18408

Weijun-H · 2025-10-31T11:49:42Z

Which issue does this PR close?

Closes Support resetting config options to defaults #18384

Rationale for this change

Without a SQL-level reset, clients that SET DataFusion options have to rebuild the session to recover defaults

What changes are included in this PR?

Extended the config macros/traits so every namespace knows how to restore default values
Added the ResetVariable

Are these changes tested?

Yes

Are there any user-facing changes?

Yes, SQL clients (including the CLI) can issue RESET

Omega359

Thank you for submitting this PR, I've left a few comments

Omega359 · 2025-10-31T13:19:38Z

datafusion/core/src/execution/context/mod.rs

        self.return_empty_dataframe()
    }

+    async fn reset_variable(&self, stmt: ResetVariable) -> Result<DataFrame> {


Seems odd to always return an empty dataframe in this function. Is that really necessary vs Result<()> ?

datafusion/common/src/config.rs

Omega359 · 2025-10-31T13:31:23Z

datafusion/common/src/config.rs

    }

+    /// Reset a configuration option back to its default value
+    pub fn reset(&mut self, key: &str) -> Result<()> {


I think overall it might be better to add a reset fn to ConfigField and update the config_namespace! macro rather than implement it like this. Just my opinion..

Omega359 · 2025-10-31T13:47:26Z

One additional thing - I didn't see anything in this PR for the unparser. It's possible that the set/unset isn't supported (and/or shouldn't be supported) there but could you take a look and see?

Weijun-H · 2025-10-31T22:23:04Z

One additional thing - I didn't see anything in this PR for the unparser. It's possible that the set/unset isn't supported (and/or shouldn't be supported) there but could you take a look and see?

@Omega359 Thanks for flagging it. I double-checked the unparser: today it simply funnels LogicalPlan::Statement through the query path, so SET/RESET plans aren’t rendered back to SQL. Supporting them would also need upstream sqlparser changes, as the AST has no native RESET node. Leaving it out of this PR was intentional given those constraints.

Omega359 · 2025-11-01T19:01:09Z

Thanks for continuing to work on this - I'll try and do an in-depth review tomorrow for this @Weijun-H.

Omega359

Looking good after a quick review. My biggest concern is the ability to reset whole sections of config - I am uncertain we want to support that.

Omega359 · 2025-11-02T17:14:51Z

datafusion/common/src/config.rs

            }
            _ => _config_err!(
-                "Config value \"{}\" not found on ConfigFileEncryptionProperties",
+                "Config value \"{}\" not found on ConfigFileDecryptionProperties",


Omega359 · 2025-11-02T17:18:44Z

datafusion/sqllogictest/test_files/set_variable.slt

+SET datafusion.runtime.memory_limit = '1M'
+
+statement ok
+RESET datafusion.runtime.memory_limit


where is the show to verify it was indeed reset?

It seems that the SHOW statement does not display runtime settings.

file #18452

Here is a hack way to test, we can ensure the below query succeed after resetting.

> set datafusion.runtime.memory_limit = '1K'; 0 row(s) fetched. Elapsed 0.001 seconds. > explain analyze select * from generate_series(1000) as t1(v1) order by v1; Not enough memory to continue external sort. Consider increasing the memory limit, or decreasing sort_spill_reservation_bytes caused by Resources exhausted: Additional allocation failed for ExternalSorterMerge[0] with top memory consumers (across reservations) as: DataFusion-Cli#89(can spill: false) consumed 0.0 B, peak 0.0 B, ExternalSorter[0]#90(can spill: true) consumed 0.0 B, peak 0.0 B, ExternalSorterMerge[0]#91(can spill: false) consumed 0.0 B, peak 0.0 B. Error: Failed to allocate additional 10.0 MB for ExternalSorterMerge[0] with 0.0 B already allocated for this reservation - 1024.0 B remain available for the total pool

Omega359 · 2025-11-02T17:22:29Z

datafusion/common/src/config.rs

+        match section {
+            "catalog" => {
+                if rem.is_empty() {
+                    self.catalog = CatalogOptions::default();


I am uncertain that this should be supported to be honest. I think that a full configuration key should be required for reset.

Omega359 · 2025-11-02T17:27:13Z

datafusion/common/src/config.rs

            }
+
+            fn reset(&mut self, key: &str) -> $crate::error::Result<()> {
+                if key.is_empty() {


This seems off. Why error if there is a key but don't if there isn't? Seems backwards?

For scalar config fields the caller already strips off the key for the field before invoking reset. At that point key only contains a nested suffix (if any).

key.is_empty() → we’re exactly at the scalar value, so resetting means “restore to default”, which we do.

non-empty key → someone tried to reset something below a scalar (e.g. foo.bar where foo is a scalar); that’s invalid, so we emit the helpful error.

Omega359 · 2025-11-02T17:27:47Z

datafusion/common/src/config.rs

+                    Ok(())
+                } else {
+                    $crate::error::_config_err!(
+                        "Config value \"{}\" not found on {}",


This error message doesn't make sense in this context. We also should have a test to cover this error

datafusion/core/src/execution/context/mod.rs

Omega359 · 2025-11-03T17:09:35Z

datafusion/core/src/execution/context/mod.rs

+            }
+            "max_temp_directory_size" => {
+                // default is 100 GB
+                const DEFAULT_MAX_TEMP_DIRECTORY_SIZE: u64 = 100 * 1024 * 1024 * 1024;


I see this const in disk_manager.rs, could we not just use that? Might need to change to pub(crate) though...

Omega359 · 2025-11-03T17:10:11Z

datafusion/core/src/execution/context/mod.rs

+            "metadata_cache_limit" => {
+                // default is 50 MB
+                const DEFAULT_METADATA_CACHE_LIMIT: usize = 50 * 1024 * 1024;
+                builder = builder.with_metadata_cache_limit(DEFAULT_METADATA_CACHE_LIMIT);


Same here in cache_manager.rs

Omega359 · 2025-11-03T17:10:41Z

Small suggestions, but otherwise LGTM, thanks!

2010YOUY01

Thank you for working on it! I haven't review the implementations yet, I'll continue soon.

2010YOUY01 · 2025-11-04T11:04:31Z

datafusion/sqllogictest/test_files/set_variable.slt

+SET datafusion.runtime.memory_limit = '1M'
+
+statement ok
+RESET datafusion.runtime.memory_limit


Here is a hack way to test, we can ensure the below query succeed after resetting.

> set datafusion.runtime.memory_limit = '1K'; 0 row(s) fetched. Elapsed 0.001 seconds. > explain analyze select * from generate_series(1000) as t1(v1) order by v1; Not enough memory to continue external sort. Consider increasing the memory limit, or decreasing sort_spill_reservation_bytes caused by Resources exhausted: Additional allocation failed for ExternalSorterMerge[0] with top memory consumers (across reservations) as: DataFusion-Cli#89(can spill: false) consumed 0.0 B, peak 0.0 B, ExternalSorter[0]#90(can spill: true) consumed 0.0 B, peak 0.0 B, ExternalSorterMerge[0]#91(can spill: false) consumed 0.0 B, peak 0.0 B. Error: Failed to allocate additional 10.0 MB for ExternalSorterMerge[0] with 0.0 B already allocated for this reservation - 1024.0 B remain available for the total pool

2010YOUY01 · 2025-11-04T11:06:47Z

datafusion/sqllogictest/test_files/set_variable.slt


 statement error Arrow error: Parser error: Invalid timezone "Asia/Taipei2": failed to parse timezone
 SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
+


Can we add some test coverage for invalid resets like
reset dataexplosion.execution.batch_size, reset datafusion.exec.batch_size, reset datafusion.execution.batches_size and datafusion.execution.batch_size.bar

Co-authored-by: Bruce Ritchie <[email protected]>

Omega359 · 2025-11-05T19:21:30Z

Looks like some tests are failing in CI

feat: add RESET statement for configuration variabless

c0af6cf

github-actions bot added sql SQL Planner logical-expr Logical plan and expressions core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) common Related to common crate labels Oct 31, 2025

Weijun-H added 2 commits October 31, 2025 13:57

chore: Update runtime desult

d8399e0

chore

0a57b6d

Omega359 reviewed Oct 31, 2025

View reviewed changes

Weijun-H added 3 commits October 31, 2025 22:54

feat: add reset functionality to configuration options

7359d0f

chore

326d804

chore: Added tests related to reset function

9cbfb17

Weijun-H force-pushed the 18384-reset-config branch from 9a18356 to 9cbfb17 Compare October 31, 2025 22:09

Weijun-H force-pushed the 18384-reset-config branch 2 times, most recently from be08bf3 to 645e40b Compare October 31, 2025 22:54

chore: Clean up

38a27d5

Weijun-H force-pushed the 18384-reset-config branch from 645e40b to 38a27d5 Compare October 31, 2025 23:04

Weijun-H added 2 commits November 1, 2025 01:27

chore: Change return type

f082aea

chore: Move reset to trait

b8a0d97

Weijun-H requested a review from Omega359 November 1, 2025 18:40

Omega359 reviewed Nov 2, 2025

View reviewed changes

Weijun-H added 2 commits November 2, 2025 22:36

chore: Improve error handling for config field resets

776311e

chore: Fix test

b931131

2010YOUY01 self-requested a review November 3, 2025 04:19

Omega359 reviewed Nov 3, 2025

View reviewed changes

datafusion/core/src/execution/context/mod.rs Outdated Show resolved Hide resolved

Omega359 reviewed Nov 3, 2025

View reviewed changes

Omega359 mentioned this pull request Nov 3, 2025

Release DataFusion 51.0.0 (Nov 2025) #17558

Open

34 tasks

2010YOUY01 reviewed Nov 4, 2025

View reviewed changes

Weijun-H and others added 4 commits November 4, 2025 21:50

Update datafusion/core/src/execution/context/mod.rs

d2bf2e9

Co-authored-by: Bruce Ritchie <[email protected]>

chore: Export default cache and directory size constants public

f960a00

chore: Added tests

625d3f3

chore: Added tests

a01905a

github-actions bot added the execution Related to the execution crate label Nov 4, 2025


		statement error Arrow error: Parser error: Invalid timezone "Asia/Taipei2": failed to parse timezone
		SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ

feat: add RESET statement for configuration variabless #18408

Are you sure you want to change the base?

feat: add RESET statement for configuration variabless #18408

Conversation

Weijun-H commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Omega359 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Omega359 commented Oct 31, 2025

Uh oh!

Weijun-H commented Oct 31, 2025

Uh oh!

Omega359 commented Nov 1, 2025

Uh oh!

Omega359 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Omega359 commented Nov 3, 2025

Uh oh!

2010YOUY01 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2010YOUY01 Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Omega359 commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Weijun-H commented Oct 31, 2025 •

edited

Loading

2010YOUY01 Nov 4, 2025 •

edited

Loading