Simplify physical expression creation API (not require schema) #8823
Conversation
github-actions
bot
added
physical-expr
alamb
changed the title
| pub fn create_physical_expr( | ||
| e: &Expr, | ||
| input_dfschema: &DFSchema, | ||
| input_schema: &Schema, |
There was a problem hiding this comment.
I wonder why it is added at the beginning. For schema itself, could it be different between input_dfschema and input_schema?
There was a problem hiding this comment.
it should be okay, if difference happen by any reason, we usually hit the error on planning Error during planning: Mismatch between schema and batches
Like below
#5695 (comment)
There was a problem hiding this comment.
Then it is weird to have duplicate parameters. I guess it might has some reason (i.e., rare cases they might be different).
@liukun4515 Do you remember it?
There was a problem hiding this comment.
I was able found an issue a year ago #4766 but later (#5695 (comment)) for similar query we started to see Error during planning: Mismatch between schema and batches. I can assume the check went to planning side.
Another suggestion on why it was done on physical expr side is we probably tried to ensure that schema are the same after all the optimization rules.
Which issue does this PR close?
Closes #.
Rationale for this change
To create physical expression we require 2 sets of schemas:
Having 2 schemas is confusing in some sense, this PR is to create physical expression based on logical schema only.
The reason the logical schema was chosen is the schema contains more information than physical schema, for instance qualifier information.
What changes are included in this PR?
Signature simplified for bunch of methods creating the physical expressions.
Are these changes tested?
Yes, existing tests
Are there any user-facing changes?
The signature for pub
create_physical_exprchanged