Minor: add method SessionStateBuilder::new_with_default_features()
#14998
Conversation
|
Why don't we just implement Default trait? |
There is some more backstory here: Basically the question is what "defaults" means -- is default an empty builder or does it have all the default functions, etc. An alternate design might be to always create SessionStateBuilder with all default functions and have a |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @shruti2522 -- this looks great. I think we just need to improve the name and it will be good to go (assuming it is ok with @berkaysynnada and other reviewers)
Could you also add a comment to SessionStateBuilder::new() here:
datafusion/datafusion/core/src/execution/session_state.rs
Lines 1022 to 1025 in b377725
Something like "to create a SessionStateBuilder with default features such as functions please see [SessionStateBuilder::new_with_default_features}
| /// .with_session_id("my_session".to_string()) | ||
| /// .build(); | ||
| /// ``` | ||
| pub fn new_with_defaults() -> Self { |
There was a problem hiding this comment.
To keep the name consistent (and make it harder to confuse this with default()) could we please name this default_features? Like:
| pub fn new_with_defaults() -> Self { | |
| pub fn new_with_default_features() -> Self { |
shruti2522
changed the title
alamb
changed the title
SessionStateBuilder::new_with_default_features()
…4998) * add func SessionStateBuilder::new_with_defaults() * update name * add comment
3 participants
Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
added method
new_with_defaultstoStreamStateBuilderAre these changes tested?
Are there any user-facing changes?