Braedon-Wooding-Displayr commented on code in PR #24043:
URL: https://github.com/apache/datafusion/pull/24043#discussion_r3740418145
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -1504,6 +1504,85 @@ fn
select_aggregate_with_group_by_with_having_using_count_star_not_in_select() {
);
}
+/// An expression containing a placeholder, written in both the SELECT list and
+/// the GROUP BY, has to be recognised as one expression the way its literal
+/// equivalent is. Otherwise the columns inside it read as ungrouped, because
the
+/// SELECT list has its placeholder types inferred and the grouping key does
not.
+#[test]
+fn select_aggregate_with_group_by_placeholder_expression() {
Review Comment:
Yes that makes perfect sense, I've done this.
##########
datafusion/sql/src/expr/order_by.rs:
##########
@@ -109,7 +109,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
))
}
e => {
- self.sql_expr_to_logical_expr(e, order_by_schema,
planner_context)?
+ let expr = self.sql_expr_to_logical_expr(
+ e,
+ order_by_schema,
+ planner_context,
+ )?;
+ let (expr, _) =
expr.infer_placeholder_types(order_by_schema)?;
Review Comment:
Great catch, I've done this.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
