vdiravka commented on a change in pull request #2286:
URL: https://github.com/apache/calcite/pull/2286#discussion_r534475582



##########
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##########
@@ -230,14 +231,27 @@ private JdbcRules() {
   public static List<RelOptRule> rules(JdbcConvention out,
       RelBuilderFactory relBuilderFactory) {
     final ImmutableList.Builder<RelOptRule> b = ImmutableList.builder();
+    b.add(JdbcToEnumerableConverterRule.create(out)

Review comment:
       What about using `rules(RelTrait in, JdbcConvention out, 
RelBuilderFactory relBuilderFactory)` here, but with null `in` value and adding 
if statement in that other overloaded method?
   It allows slightly avoid duplication of code

##########
File path: 
core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcToEnumerableConverterRule.java
##########
@@ -30,9 +30,9 @@
  */
 public class JdbcToEnumerableConverterRule extends ConverterRule {
   /** Creates a JdbcToEnumerableConverterRule. */
-  public static JdbcToEnumerableConverterRule create(JdbcConvention out) {
+  public static JdbcToEnumerableConverterRule create(JdbcConvention in) {

Review comment:
       This convention is `in` for rule, but `out` for planner (or who use this 
rule), am I right?




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