Pruning serialization #1941
Pruning serialization #1941
Conversation
| @@ -143,6 +145,7 @@ impl PruningPredicate { | |||
| schema, | |||
| predicate_expr, | |||
| required_columns, | |||
| logical_expr: expr.clone(), | |||
There was a problem hiding this comment.
It might be worth considering changing the signature of this function from `
pub fn try_new(expr: &Expr, schema: SchemaRef) -> Result<Self> {to take the Expr directly rather than clone within
pub fn try_new(expr: Expr, schema: SchemaRef) -> Result<Self> {The rationale would be if the caller already has a Expr they could pass it in rather than forcing a copy.
There was a problem hiding this comment.
We could always do that as a follow on PR as well
There was a problem hiding this comment.
Makes sense. I should have some time tomorrow to see if I can do this relatively easily. If it turns out to be more involved then we can create a follow up task for it.
There was a problem hiding this comment.
Thanks @thinkharderdev -- to keep the code flowing I am going to merge this PR as is and we can do the cleanup in a follow on PR. Let me know if you don't get a chance and I can file a follow on ticket to do so
|
This PR seems to have a logical conflict with #1887 -- I am working to fix that |
|
Fix in #1958 |
Which issue does this PR close?
Closes #.
Rationale for this change
Currently, we don't preserve pruning predicates in
ParquetExecwhen serializing for Ballista. This was probably the case because thePruningPredicaterequires the original logicalExprto reconstruct. This PR preserves the originalExprinPruningPredicateso we can serialize it.What changes are included in this PR?
Exprin thePruningPredicatestruct.2.Exprin the protobuf message which encodes theParquetExecPruninPredicatewhen deserializing theParquetExec.Are there any user-facing changes?
No
No