zabetak commented on a change in pull request #1884:
URL: https://github.com/apache/calcite/pull/1884#discussion_r422871750



##########
File path: core/src/main/java/org/apache/calcite/rel/RelCollationTraitDef.java
##########
@@ -69,10 +70,14 @@ public RelNode convert(
       return null;
     }
 
-    // Create a logical sort, then ask the planner to convert its remaining
-    // traits (e.g. convert it to an EnumerableSortRel if rel is enumerable
-    // convention)
-    final Sort sort = LogicalSort.create(rel, toCollation, null, null);
+    // Create a sort operator based on given convention
+    Convention toConvention = rel.getConvention();
+    RelBuilder builder =
+        RelFactories.LOGICAL_BUILDER.create(rel.getCluster(), null);
+    RelNode sort = builder.push(rel)
+                          .adoptConvention(toConvention)
+                          .sort(toCollation)
+                          .build();

Review comment:
       I guess I missed again this discussion (and still don't find it :)) 
apologies for that. Just to be sure we are on the same page I am just talking 
for this piece of code where we are only doing enforcement. I'm not questioning 
the other scenarios. 




----------------------------------------------------------------
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:
[email protected]


Reply via email to