[MINOR]: Update create_window_expr to refer only input schema #8945
Conversation
There was a problem hiding this comment.
THank you @mustafasrepo -- this makes sense to me. cc @comphead
|
I recommend we wait for another day to see if @comphead has any additional comments and then merge this PR |
| @@ -1552,10 +1549,10 @@ pub fn create_window_expr_with_name( | |||
| e: &Expr, | |||
| name: impl Into<String>, | |||
| logical_input_schema: &DFSchema, | |||
| physical_input_schema: &Schema, | |||
There was a problem hiding this comment.
Thanks @mustafasrepo
I'm still thinking if we need to have a physical schema? This is misleading a bit, we always need to keep in mind why the schemas are not the same, what is their difference, etc.
Basically we have a check earlier in the planner that asserts equality for the schema from batch and from the planner.
https://github.com/search?q=repo%3Aapache%2Farrow-datafusion%20mismatch%20between&type=code
There was a problem hiding this comment.
Thanks @comphead, It seems that we really don't need physical_input_schema. Removed it, thanks for the suggestion.
Which issue does this PR close?
Closes #.
Rationale for this change
With the PR8920
create_window_exprfunction refers to window_schema to get its window expr output type. However, at this stage it should calculate its output type usinginput_schema. With this PR, function only refers to its input schema.What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?