You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/03/31 19:05:09 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #5814: minor(sqlparser): encapsulate PlanerContext, reduce some clones
alamb opened a new pull request, #5814:
URL: https://github.com/apache/arrow-datafusion/pull/5814
# Which issue does this PR close?
related to #5637 , but really a Friday afternoon project that has bothered me for a whle
# Rationale for this change
The PlannerContext in the SQL planner has a map of `LogicalPlans` and it is cloned during planning (which deep clones the values 😱 ). Also, the LogicalPlan is cloned on use anyways.
# What changes are included in this PR?
1. Make the fields of `PlannerContext` non pub
2.
# Are these changes tested?
Covered by existing tests
# Are there any user-facing changes?
No
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #5814: minor(sqlparser): encapsulate PlanerContext, reduce some clones
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5814:
URL: https://github.com/apache/arrow-datafusion/pull/5814#issuecomment-1494870503
Thanks @avantgardnerio !
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5814: minor(sqlparser): encapsulate PlanerContext, reduce some clones
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5814:
URL: https://github.com/apache/arrow-datafusion/pull/5814#discussion_r1154805330
##########
datafusion/sql/src/relation/mod.rs:
##########
@@ -33,7 +33,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// normalize name and alias
let table_ref = self.object_name_to_table_reference(name)?;
let table_name = table_ref.to_string();
- let cte = planner_context.ctes.get(&table_name);
+ let cte = planner_context.get_cte(&table_name);
Review Comment:
this is the use of `cte`s -- and 4 lines below it is immediately `clone()`d
##########
datafusion/sql/src/planner.rs:
##########
@@ -100,15 +102,52 @@ impl PlannerContext {
}
}
- /// Create a new PlannerContext with provided prepare_param_data_types
- pub fn new_with_prepare_param_data_types(
+ /// Update the PlannerContext with provided prepare_param_data_types
+ pub fn with_prepare_param_data_types(
+ mut self,
prepare_param_data_types: Vec<DataType>,
) -> Self {
- Self {
- prepare_param_data_types,
- ctes: HashMap::new(),
- outer_query_schema: None,
- }
+ self.prepare_param_data_types = prepare_param_data_types;
+ self
+ }
+
+ // return a reference to the outer queries schema
+ pub fn outer_query_schema(&self) -> Option<&DFSchema> {
+ self.outer_query_schema.as_ref()
+ }
+
+ /// sets the outer query schema, returning the existing one, if
Review Comment:
I think having accessors also is a good place to put documentation about what they do
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb merged pull request #5814: minor(sqlparser): encapsulate PlanerContext, reduce some clones
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #5814:
URL: https://github.com/apache/arrow-datafusion/pull/5814
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org