stuartcarnie commented on code in PR #5307:
URL: https://github.com/apache/arrow-datafusion/pull/5307#discussion_r1112374453
##########
datafusion/core/tests/sqllogictests/test_files/order.slt:
##########
@@ -258,6 +258,32 @@ ORDER BY time;
statement error DataFusion error: This feature is not implemented: SORT BY
select * from t SORT BY time;
+
+# distinct on a column not in the select list should not work
+statement error For SELECT DISTINCT, ORDER BY expressions time must appear in
select list
+SELECT DISTINCT value FROM t ORDER BY time;
+
+# distinct on an expression of a column not in the select list should not work
+statement error For SELECT DISTINCT, ORDER BY expressions time must appear in
select list
+SELECT DISTINCT date_trunc('hour', time) FROM t ORDER BY time;
+
+# distinct on a column that is in the select list but aliasted should work
+query I
+SELECT DISTINCT time as "first_seen" FROM t ORDER BY "first_seen";
+----
+2022-01-01T00:00:30
+2022-01-01T01:00:10
+2022-01-02T00:00:20
+
+# distinct on a column that is in the select list, but aliased (though
+# the reference is to original expr) should work
+query I
+SELECT DISTINCT time as "first_seen" FROM t ORDER BY time;
+----
+2022-01-01T00:00:30
+2022-01-01T01:00:10
+2022-01-02T00:00:20
+
Review Comment:
```suggestion
# distinct on a column that is in the select list, but aliased (though
# the reference is its ordinal position) should work
query I
SELECT DISTINCT time as "first_seen" FROM t ORDER BY 1;
----
2022-01-01T00:00:30
2022-01-01T01:00:10
2022-01-02T00:00:20
```
I don't believe your PR is affected by that feature, however, I notice there
isn't a test for ordering by ordinal position, so is it worth adding one?
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -431,7 +497,9 @@ impl LogicalPlanBuilder {
.map(|f| Expr::Column(f.qualified_column()))
.collect();
- let plan = Self::add_missing_columns(self.plan, &missing_cols)?;
+ let is_distinct = false;
+ let plan = Self::add_missing_columns(self.plan, &missing_cols,
is_distinct)?;
+ println!("Result plan is\n{plan:#?}");
Review Comment:
```suggestion
```
Left over debug code?
--
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]
