[GitHub] [calcite] jinxing64 commented on a change in pull request #1941: [CALCITE-3952] Improve SortRemoveRule to remove Sort based on rowcount (Vineet Garg)

[GitBox <gi...@apache.org>]

jinxing64 commented on a change in pull request #1941:
URL: https://github.com/apache/calcite/pull/1941#discussion_r415249381



##########
File path: core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRule.java
##########
@@ -63,5 +83,27 @@ public SortRemoveRule(RelBuilderFactory relBuilderFactory) {
         .getTrait(RelCollationTraitDef.INSTANCE);
     final RelTraitSet traits = sort.getInput().getTraitSet().replace(collation);
     call.transformTo(convert(sort.getInput(), traits));
+    return true;
+  }
+
+  // Sort is removed if all of following conditions are met
+  // 1. Sort's input is guaranteed to produce at most one row
+  // 2. If fetch is greater than 0
+  // 3. If offset is less than 1
+  public static boolean shouldRemoveSortBasedOnRowCount(final RelMetadataQuery mq,
+      final RelNode input, int fetch, int offset) {
+    Double maxRowCount = mq.getMaxRowCount(input);
+    if (maxRowCount != null && maxRowCount <= 1) {
+      if (fetch == 0) {
+        return false;
+      } else if (fetch > 0) {
+        if (offset < 1) {
+          return true;
+        }
+      } else {
+        return true;

Review comment:
       What if `fetch = -1` and `offset > 1`, should it return `true` ? If no, do we have test to cover ?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org