Omega359 commented on code in PR #18408:
URL: https://github.com/apache/datafusion/pull/18408#discussion_r2481429316
##########
datafusion/common/src/config.rs:
##########
@@ -1174,6 +1174,48 @@ impl ConfigOptions {
e.0.set(key, value)
}
+ /// Reset a configuration option back to its default value
+ pub fn reset(&mut self, key: &str) -> Result<()> {
Review Comment:
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..
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -1104,6 +1108,36 @@ impl SessionContext {
self.return_empty_dataframe()
}
+ async fn reset_variable(&self, stmt: ResetVariable) -> Result<DataFrame> {
Review Comment:
Seems odd to always return an empty dataframe in this function. Is that
really necessary vs `Result<()>` ?
##########
datafusion/common/src/config.rs:
##########
@@ -1174,6 +1174,48 @@ impl ConfigOptions {
e.0.set(key, value)
}
+ /// Reset a configuration option back to its default value
+ pub fn reset(&mut self, key: &str) -> Result<()> {
+ let Some((prefix, rest)) = key.split_once('.') else {
+ return _config_err!("could not find config namespace for key
\"{key}\"");
+ };
+
+ if prefix != "datafusion" {
+ return _config_err!("Could not find config namespace
\"{prefix}\"");
+ }
+
+ let entries = self.entries();
+ if !entries.iter().any(|entry| entry.key == key) {
+ return _config_err!("Config value \"{key}\" not found on
ConfigOptions");
+ }
+
+ let mut skip_keys: HashSet<String> = HashSet::from([key.to_string()]);
+ if rest == "optimizer.enable_dynamic_filter_pushdown" {
+ skip_keys.insert(
+
"datafusion.optimizer.enable_topk_dynamic_filter_pushdown".to_string(),
+ );
+ skip_keys.insert(
+
"datafusion.optimizer.enable_join_dynamic_filter_pushdown".to_string(),
+ );
+ }
+
+ let mut new_options = ConfigOptions::default();
+ new_options.extensions = self.extensions.clone();
+
+ for entry in entries {
Review Comment:
While this works I wonder if there is a way to do this that doesn't require
iterating over every configuration just to not set the one key that we're
resetting.
--
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]
