alamb commented on code in PR #14935:
URL: https://github.com/apache/datafusion/pull/14935#discussion_r1975987954
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -1144,7 +1146,9 @@ impl SessionStateBuilder {
mut self,
expr_planners: Vec<Arc<dyn ExprPlanner>>,
) -> Self {
- self.expr_planners = Some(expr_planners);
+ if self.expr_planners.is_none() {
Review Comment:
By putting these checks here I think the API will be quite confusing to use
-- it means that calling `with_expr_planners` and the other methods below will
only set the fields if there is no existing field set and silently ignore the
argument if there is already planners specified
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -1081,6 +1081,8 @@ impl SessionStateBuilder {
/// Create default builder with defaults for table_factories, file
formats, expr_planners and builtin
/// scalar, aggregate and windows functions.
+ /// For each setter method call, default values will only be created and
set if the corresponding
+ /// field is currently None, otherwise the existing value will be
preserved.
pub fn with_default_features(self) -> Self {
Review Comment:
Thanks @irenjj
I thought the idea was that `with_default_features` would be *additive* to
the existing set of features
That is, instead of replacing any existing factors/formats/etc this methods
would *append* the default features
So instead of
```rust
self.with_table_factories(SessionStateDefaults::default_table_factories())
```
Something more like
```rust
let mut table_factories =
self.with_table_factories.take().unwrap_or_default();
table_factories.extend(SessionStateDefaults::default_table_factories())
self.table_factories = Some(table_factories)
```
🤔
--
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]
