You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/06/10 00:25:47 UTC

[GitHub] [calcite] hsyuan opened a new pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

hsyuan opened a new pull request #2017:
URL: https://github.com/apache/calcite/pull/2017


   In addition, add code snippet to demonstrate how to generate IndexScan on
   demand by passing required collation through TableScan.
   
   CALCITE-4018


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



[GitHub] [calcite] amaliujia commented on a change in pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #2017:
URL: https://github.com/apache/calcite/pull/2017#discussion_r437796059



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }
+      }
+      if (!ordering.isOrdered(tuples)) {
+        return null;

Review comment:
       Could you also explain which this check?

##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }

Review comment:
       Can you add comments to describe what this for loop is doing? Sorry but I had a hard time to understand it.




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



[GitHub] [calcite] chunweilei commented on a change in pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2017:
URL: https://github.com/apache/calcite/pull/2017#discussion_r437833608



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }
+      }
+      if (!ordering.isOrdered(tuples)) {
+        return null;

Review comment:
       If the tuples are not sorted, we still have to add a sort on it. 




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



[GitHub] [calcite] hsyuan commented on a change in pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #2017:
URL: https://github.com/apache/calcite/pull/2017#discussion_r437834172



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }
+      }
+      if (!ordering.isOrdered(tuples)) {
+        return null;

Review comment:
       @chunweilei Are you explaining to @amaliujia or just pointing out a potential bug? :)




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



[GitHub] [calcite] amaliujia commented on a change in pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #2017:
URL: https://github.com/apache/calcite/pull/2017#discussion_r437863902



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }
+      }
+      if (!ordering.isOrdered(tuples)) {
+        return null;

Review comment:
       Thanks! Now I see it better!




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



[GitHub] [calcite] chunweilei commented on a change in pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2017:
URL: https://github.com/apache/calcite/pull/2017#discussion_r437853345



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }
+      }
+      if (!ordering.isOrdered(tuples)) {
+        return null;

Review comment:
       > @chunweilei Are you explaining to @amaliujia or just pointing out a potential bug? :)
   
   I am explaining to @amaliujia ~~




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



[GitHub] [calcite] hsyuan commented on a change in pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #2017:
URL: https://github.com/apache/calcite/pull/2017#discussion_r437798973



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableValues.java
##########
@@ -70,6 +74,32 @@ public static EnumerableValues create(RelOptCluster cluster,
     return new EnumerableValues(getCluster(), rowType, tuples, traitSet);
   }
 
+  @Override public RelNode passThrough(final RelTraitSet required) {
+    RelCollation collation = required.getCollation();
+    if (collation == null || collation.isDefault()) {
+      return null;
+    }
+    if (tuples.size() > 1) {
+      Ordering<List<RexLiteral>> ordering = null;
+      for (RelFieldCollation fc : collation.getFieldCollations()) {
+        Ordering<List<RexLiteral>> comparator = RelMdCollation.comparator(fc);
+        if (ordering == null) {
+          ordering = comparator;
+        } else {
+          ordering = ordering.compound(comparator);
+        }

Review comment:
       Sure, I added comments.




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



[GitHub] [calcite] hsyuan merged pull request #2017: [CALCITE-4018] Support trait propagation for EnumerableValues

Posted by GitBox <gi...@apache.org>.
hsyuan merged pull request #2017:
URL: https://github.com/apache/calcite/pull/2017


   


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