rubenada commented on code in PR #4181:
URL: https://github.com/apache/calcite/pull/4181#discussion_r1946175710


##########
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##########
@@ -795,6 +801,122 @@ private static void shiftMapping(Map<Integer, Integer> 
mapping, int startIndex,
     return null;
   }
 
+  private @Nullable Frame decorrelateFetchOneSort(Sort sort, final Frame 
frame) {
+    final Map<Integer, Integer> mapOldToNewOutputs = new HashMap<>();
+    final NavigableMap<CorDef, Integer> corDefOutputs = new TreeMap<>();
+    //
+    // Rewrite logic:
+    //
+    // If sorted with no OFFSET and FETCH = 1, and only one collation field,
+    // rewrite the Sort as Aggregate using MIN/MAX function.
+    // Example:
+    //  Sort(sort0=[$0], dir0=[ASC], fetch=[1])
+    //   input
+    // Rewrite to:
+    //  Aggregate(group=(corVar), agg=[min($0))
+    //
+    // Note: MIN/MAX is not strictly equivalent to LIMIT 1. When the input has 
0 rows,
+    // MIN/MAX returns NULL, while LIMIT 1 returns 0 rows.
+    // However, in the decorrelate, we add correlated variables to the group 
list
+    // to ensure equivalence when Correlate is transformed to Join. When the 
group list
+    // is non-empty, MIN/MAX will also return 0 rows if input with 0 
rows.(This behavior
+    // also leads to the issue described in [CALCITE-5743]) So in this case, 
the transformation
+    // is legal.
+    if (sort.getCollation().getFieldCollations().size() == 1
+        && sort.getRowType().getFieldCount() == 1
+        && !frame.corDefOutputs.isEmpty()) {
+      RelFieldCollation collation = 
Util.first(sort.getCollation().getFieldCollations());
+
+      SqlAggFunction aggFunction;
+      switch (collation.getDirection()) {
+      case ASCENDING:
+      case STRICTLY_ASCENDING:
+        aggFunction = SqlStdOperatorTable.MIN;
+        break;
+      case DESCENDING:
+      case STRICTLY_DESCENDING:
+        aggFunction = SqlStdOperatorTable.MAX;
+        break;
+      default:
+        return null;
+      }
+
+      final int newIdx = 
requireNonNull(frame.oldToNewOutputs.get(collation.getFieldIndex()));
+      RelBuilder.AggCall aggCall = relBuilder.push(frame.r)

Review Comment:
   This was my exact same thought after a quick look at the PR... the proposed 
conversion is 100% equivalent in all cases, also when nulls are involved? What 
about the original Sort is nulls-first and the relevant field has null values?



-- 
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]

Reply via email to