docs: Window::try_new_with_schema with a descriptive error message#17926
docs: Window::try_new_with_schema with a descriptive error message#17926alamb merged 1 commit intoapache:mainfrom
Window::try_new_with_schema with a descriptive error message#17926Conversation
| /// Create a new window function using the provided schema to avoid the overhead of | ||
| /// building the schema again when the schema is already known. | ||
| /// | ||
| /// This method should only be called when you are absolutely sure that the schema being | ||
| /// provided is correct for the window function. If in doubt, call [try_new](Self::try_new) instead. |
There was a problem hiding this comment.
I reuse the docs from Aggregate::try_new_with_schema
datafusion/datafusion/expr/src/logical_plan/plan.rs
Lines 3451 to 3456 in 9e8ec54
| if window_expr.len() != schema.fields().len() - input.schema().fields().len() { | ||
| let input_fields_count = input.schema().fields().len(); | ||
| if schema.fields().len() != input_fields_count + window_expr.len() { | ||
| return plan_err!( | ||
| "Window has mismatch between number of expressions ({}) and number of fields in schema ({})", | ||
| window_expr.len(), | ||
| schema.fields().len() - input.schema().fields().len() | ||
| "Window schema has wrong number of fields. Expected {} got {}", | ||
| input_fields_count + window_expr.len(), | ||
| schema.fields().len() |
There was a problem hiding this comment.
I follow the error message from Aggregate::try_new_with_schema.
datafusion/datafusion/expr/src/logical_plan/plan.rs
Lines 3470 to 3477 in 9e8ec54
Yes, we currently have no test covering the failure case of |
Maybe it is worth filing a ticket to track However, I think this particular error message is most likely to show up if someone creates an incorrect plan programatically rather than directly by a higher API so maybe it isn't critical |
4 participants
Which issue does this PR close?
Window::try_new_with_schema#17912.Rationale for this change
Window::try_new_with_schemais missing doc string with unclear error message.What changes are included in this PR?
Window::try_new_with_schema.Are these changes tested?
No. I think this is a just a small docs change.
Are there any user-facing changes?
No.