mihaibudiu commented on code in PR #4933:
URL: https://github.com/apache/calcite/pull/4933#discussion_r3437773672
##########
babel/src/test/resources/sql/select.iq:
##########
@@ -332,4 +332,82 @@ from emp e join dept d on e.deptno = d.deptno;
SELECT * REPLACE list contains unknown column(s): DEPTNO
!error
+# [CALCITE-5406] Support the SELECT DISTINCT ON statement for PostgreSQL
dialect
Review Comment:
I think I made this suggestion before: if these were validated on postgres,
pleas say so in the comment, to spare future reviewers the need to validate
these. I have seen tests with incorrect results in the past.
##########
core/src/main/java/org/apache/calcite/sql/SqlSelect.java:
##########
@@ -160,12 +183,23 @@ public SqlSelect(SqlParserPos pos,
case 10:
fetch = operand;
break;
+ case 11:
+ hints = (SqlNodeList) operand;
Review Comment:
this looks like a bug you are fixing
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -1021,6 +1042,77 @@ private void distinctify(
rel.getRowType().getFieldNames(), ImmutableSet.of()), false);
}
+ /**
+ * Converts a SELECT DISTINCT ON clause into a relational expression
+ * using ROW_NUMBER() window function.
+ */
+ private void convertDistinctOn(Blackboard bb, SqlSelect select,
+ List<RelFieldCollation> collationList) {
Review Comment:
what if the SELECT already has a window query?
Is there a test case for that combination?
##########
core/src/main/java/org/apache/calcite/sql/SqlSelect.java:
##########
@@ -327,4 +361,16 @@ public boolean hasWhere() {
public boolean isKeywordPresent(SqlSelectKeyword targetKeyWord) {
return getModifierNode(targetKeyWord) != null;
}
+
+ public boolean isDistinctOn() {
+ return distinctOn != null && !distinctOn.isEmpty();
+ }
+
+ public @Nullable SqlNodeList getDistinctOn() {
+ return distinctOn;
+ }
+
+ public void setDistinctOn(@Nullable SqlNodeList distinctOn) {
Review Comment:
is this needed?
In general immutable data structures are preferred.
--
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]
