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

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

Looks like some tests are failing in CI

Omega359 · 2025-11-09T16:56:44Z

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

+            state.register_udf(udf)?;
+        }
+
+        drop(state);


Why are you explicitly dropping state here? I can see no reason to do this.

Removed it because it is not needed.

Omega359

I think this is ready to go, thanks for all the work on this!

2010YOUY01

I think this is a really nice feature, thank you.

2010YOUY01 · 2025-11-17T07:51:51Z

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

                let limit = Self::parse_memory_limit(value)?;
                builder.with_metadata_cache_limit(limit)
            }
            _ => return plan_err!("Unknown runtime configuration: {variable}"),


Suggested change

_ => return plan_err!("Unknown runtime configuration: {variable}"),

// Remember to update `reset_runtime_variable()` when adding new options

2010YOUY01 · 2025-11-17T08:02:01Z

datafusion/sql/src/parser.rs

    }

+    /// Parse a SQL `RESET`
+    pub fn parse_reset(&mut self) -> Result<Statement, DataFusionError> {


Just curious -- why for certain statements we decide to implement in-house parsing, instead of using sqlparser dependency?

sqlparser only supports ALTER ROLE ... RESET now, so we have to implement from datafusion side.

2010YOUY01 · 2025-11-17T08:02:59Z

datafusion/sql/src/statement.rs

+                let variable = object_name_to_string(&variable);
+                let mut variable_lower = variable.to_lowercase();
+
+                if variable_lower == "timezone" || variable_lower == "time.zone" {


Could you add some comment to explain this special case?

Co-authored-by: Bruce Ritchie <bruce.ritchie@veeva.com>

Weijun-H · 2025-11-17T19:25:41Z

Thanks @Omega359 and @2010YOUY01 for reviewing !

## Which issue does this PR close?  - Closes apache#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 --------- Co-authored-by: Bruce Ritchie <bruce.ritchie@veeva.com>

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

Omega359 reviewed Oct 31, 2025

View reviewed changes

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 3 times, most recently from 645e40b to 38a27d5 Compare October 31, 2025 23:04

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

Omega359 reviewed Nov 2, 2025

View reviewed changes

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

Closed

37 tasks

2010YOUY01 reviewed Nov 4, 2025

View reviewed changes

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

Weijun-H force-pushed the 18384-reset-config branch from 8945228 to 4469cfc Compare November 8, 2025 09:38

Weijun-H requested review from 2010YOUY01 and Omega359 November 8, 2025 10:12

Omega359 reviewed Nov 9, 2025

View reviewed changes

Weijun-H force-pushed the 18384-reset-config branch 2 times, most recently from 76551fb to b8ca1cb Compare November 9, 2025 17:49

Weijun-H force-pushed the 18384-reset-config branch from b8ca1cb to dab3922 Compare November 11, 2025 21:17

Weijun-H requested a review from Omega359 November 11, 2025 21:17

Weijun-H force-pushed the 18384-reset-config branch 2 times, most recently from 22f83c9 to 66d1b48 Compare November 15, 2025 21:41

Omega359 approved these changes Nov 16, 2025

View reviewed changes

2010YOUY01 approved these changes Nov 17, 2025

View reviewed changes

Weijun-H and others added 17 commits November 17, 2025 20:09

feat: add RESET statement for configuration variabless

bf40be2

chore: Update runtime desult

5e2cbf6

chore

2617a0c

feat: add reset functionality to configuration options

239def2

chore

643e4d6

chore: Added tests related to reset function

a6eff6f

chore: Clean up

d1d627d

chore: Change return type

056339e

chore: Move reset to trait

ade3a70

chore: Improve error handling for config field resets

ba2edf1

chore: Fix test

16a7813

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

575d6ef

Co-authored-by: Bruce Ritchie <bruce.ritchie@veeva.com>

chore: Export default cache and directory size constants public

40ed8be

chore: Added tests

e341427

chore: Added tests

6d689a8

chore

1a6ebc4

chore

2bfb876

Weijun-H force-pushed the 18384-reset-config branch from 66d1b48 to 2bfb876 Compare November 17, 2025 18:10

Weijun-H added this pull request to the merge queue Nov 17, 2025

Merged via the queue into apache:main with commit 71fcd03 Nov 17, 2025
32 checks passed


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


	_ => return plan_err!("Unknown runtime configuration: {variable}"),
	// Remember to update `reset_runtime_variable()` when adding new options

feat: add RESET statement for configuration variabless #18408

feat: add RESET statement for configuration variabless #18408

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Omega359 left a comment

Choose a reason for hiding this comment

Uh oh!

2010YOUY01 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

Weijun-H commented Oct 31, 2025 •

edited

Loading

2010YOUY01 Nov 4, 2025 •

edited

Loading