[GitHub] [calcite] chunweilei commented on issue #1167: [CALCITE-3009] It should fail if there are duplicate keys in a .xml file

[GitBox]

chunweilei commented on issue #1167: [CALCITE-3009] It should fail if there are 
duplicate keys in a .xml file
URL: https://github.com/apache/calcite/pull/1167#issuecomment-484779656
 
 
   Update the PR including:
   * Add a `validate` method to check all test cases when loading the document
   * Change the commit message to "Duplicate test cases in the .xml file"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276896982
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   hi @pengzhiwei2018 Thank you very much for your help.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei edited a comment on issue #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei edited a comment on issue #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#issuecomment-484757984
 
 
   Update the PR including:
   * Rename `removeJoin` to `canRemoveJoin`
   * Use `RexUtil#shift` to transform expressions
   * Rename 'transformAggregateCall' to 'shift'
   * Simplify some `if` conditions to `leftKeys.stream().anyMatch(s -> s >= 
leftBottomChildSize)`
   * Add some comments
   
   @hsyuan Please review the PR when you are free. Thanks a lot.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei commented on issue #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on issue #1160: [CALCITE-2712] Add rule to eliminate 
unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#issuecomment-484757984
 
 
   Update the PR including:
   * Rename `removeJoin` to `canRemoveJoin`
   * Use `RexUtil#shift` to transform expressions
   * Rename 'transformAggregateCall' to 'shift'
   * Simplify some `if` conditions to `leftKeys.stream().anyMatch(s -> s >= 
leftBottomChildSize)`
   * Add some 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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887623
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Project}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class ProjectJoinJoinRemoveRule extends RelOptRule {
+  public static final ProjectJoinJoinRemoveRule INSTANCE
+  = new ProjectJoinJoinRemoveRule(LogicalProject.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates a ProjectJoinJoinRemoveRule. */
+  public ProjectJoinJoinRemoveRule(
+  Class projectClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(projectClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Project project = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = 
bottomJoin.getLeft().getRowType().getFieldCount();
+
+// Check whether the project uses unexpected columns.
+if (!RelOptUtil.removeJoin(project, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
+  return;
+}
+
+// Check whether left join keys in top join and bottom join are equal.
+final List leftChildKeys = new ArrayList<>();
+final List rightChildKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(bottomJoin.getLeft(), bottomJoin.getRight(),
+bottomJoin.getCondition(), leftChildKeys, rightChildKeys,
+new ArrayList<>());
+if (!leftKeys.equals(leftChildKeys)) {
+  return;
+}
+
+// Make sure that right keys of bottom join are unique.
+final ImmutableBitSet.Builder columns = ImmutableBitSet.builder();
+rightChildKeys.forEach(key -> columns.set(key));
+final RelMetadataQuery mq = call.getMetadataQuery();
+if (!mq.areColumnsUnique(bottomJoin.getRight(), columns.build())) {
+

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887497
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Aggregate}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class AggregateJoinJoinRemoveRule extends RelOptRule {
+  public static final AggregateJoinJoinRemoveRule INSTANCE
+  = new AggregateJoinJoinRemoveRule(LogicalAggregate.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates an AggregateJoinJoinRemoveRule. */
+  public AggregateJoinJoinRemoveRule(
+  Class aggregateClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(aggregateClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Aggregate aggregate = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = bottomJoin.getLeft().getRowType()
+.getFieldCount();
+
+// Check whether the aggregate uses unexpected columns.
+if (!RelOptUtil.removeJoin(aggregate, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
 
 Review comment:
   The `if` condition can be simplified to `leftKeys.stream().anyMatch(s -> s 
>= leftBottomChildSize)`.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887530
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Project}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class ProjectJoinJoinRemoveRule extends RelOptRule {
+  public static final ProjectJoinJoinRemoveRule INSTANCE
+  = new ProjectJoinJoinRemoveRule(LogicalProject.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates a ProjectJoinJoinRemoveRule. */
+  public ProjectJoinJoinRemoveRule(
+  Class projectClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(projectClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Project project = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = 
bottomJoin.getLeft().getRowType().getFieldCount();
+
+// Check whether the project uses unexpected columns.
+if (!RelOptUtil.removeJoin(project, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
+  return;
+}
+
+// Check whether left join keys in top join and bottom join are equal.
+final List leftChildKeys = new ArrayList<>();
+final List rightChildKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(bottomJoin.getLeft(), bottomJoin.getRight(),
+bottomJoin.getCondition(), leftChildKeys, rightChildKeys,
+new ArrayList<>());
+if (!leftKeys.equals(leftChildKeys)) {
+  return;
+}
+
+// Make sure that right keys of bottom join are unique.
+final ImmutableBitSet.Builder columns = ImmutableBitSet.builder();
+rightChildKeys.forEach(key -> columns.set(key));
+final RelMetadataQuery mq = call.getMetadataQuery();
+if (!mq.areColumnsUnique(bottomJoin.getRight(), columns.build())) {
+

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887509
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Aggregate}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class AggregateJoinJoinRemoveRule extends RelOptRule {
+  public static final AggregateJoinJoinRemoveRule INSTANCE
+  = new AggregateJoinJoinRemoveRule(LogicalAggregate.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates an AggregateJoinJoinRemoveRule. */
+  public AggregateJoinJoinRemoveRule(
+  Class aggregateClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(aggregateClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Aggregate aggregate = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = bottomJoin.getLeft().getRowType()
+.getFieldCount();
+
+// Check whether the aggregate uses unexpected columns.
+if (!RelOptUtil.removeJoin(aggregate, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
+  return;
+}
+
+// Check whether left join keys in top join and bottom join are equal.
+final List leftChildKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(bottomJoin.getLeft(), bottomJoin.getRight(),
+bottomJoin.getCondition(), leftChildKeys, new ArrayList<>(),
+new ArrayList<>());
+if (!leftKeys.equals(leftChildKeys)) {
+  return;
+}
+
+// Adjust group keys according to offset.
+int offset = bottomJoin.getRight().getRowType().getFieldCount();
+final ImmutableBitSet.Builder groupKey = ImmutableBitSet.builder();
+aggregate.getGroupSet().forEach(
+i -> groupKey.set(i >= leftBottomChildSize ? i - offset : i));
+
+final RexBuilder rexBuilder = topJoin.getCluster().getRexBuilder();
+final RelBuilder relBuilder = call.builder();
+
+relBuilder.push(bottomJoin.getLeft()).push(topJoin.getRight())
+.join(topJoin.getJoinType

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887310
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -735,6 +737,95 @@ public static RelNode createCastRel(
 }
   }
 
+  /** Transforms the aggregate call according to the limit and offset. */
+  public static List transformAggregateCall(
+  List aggregateCalls, int limit, int offset) {
+// Adjust arguments of the aggregate call.
+final List newAggregateCall = new ArrayList<>();
+for (AggregateCall aggregateCall : aggregateCalls) {
+  final List args = new ArrayList<>();
+  aggregateCall.getArgList()
+  .forEach(i -> args.add(i >= limit ? i - offset : i));
+
+  newAggregateCall.add(AggregateCall
+  .create(aggregateCall.getAggregation(), aggregateCall.isDistinct(),
+  aggregateCall.isApproximate(), aggregateCall.ignoreNulls(), args,
+  aggregateCall.filterArg, aggregateCall.getCollation(),
+  aggregateCall.getType(), aggregateCall.getName()));
+}
+return newAggregateCall;
+  }
+
+  /** Transforms the condition according to the limit and offset. */
+  public static RexNode transformCondition(RexNode condition,
+  RexBuilder rexBuilder, int limit, int offset) {
+RexShuttle shuttle = new RexShuttle() {
+  @Override public RexNode visitInputRef(RexInputRef inputRef) {
+int index = inputRef.getIndex();
+if (index >= limit) {
+  return rexBuilder.makeInputRef(inputRef.getType(), index - offset);
+}
+return inputRef;
+  }
+};
+return condition.accept(shuttle);
+  }
+
+  /**
+   * Whether it can remove join according to the aggregate.
+   *
+   * @param aggregate  Aggregate
+   * @param lowerLimit lower limit (included)
+   * @param upperLimit upper limit (excluded)
 
 Review comment:
   Got it. Will explain it more clear.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887310
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -735,6 +737,95 @@ public static RelNode createCastRel(
 }
   }
 
+  /** Transforms the aggregate call according to the limit and offset. */
+  public static List transformAggregateCall(
+  List aggregateCalls, int limit, int offset) {
+// Adjust arguments of the aggregate call.
+final List newAggregateCall = new ArrayList<>();
+for (AggregateCall aggregateCall : aggregateCalls) {
+  final List args = new ArrayList<>();
+  aggregateCall.getArgList()
+  .forEach(i -> args.add(i >= limit ? i - offset : i));
+
+  newAggregateCall.add(AggregateCall
+  .create(aggregateCall.getAggregation(), aggregateCall.isDistinct(),
+  aggregateCall.isApproximate(), aggregateCall.ignoreNulls(), args,
+  aggregateCall.filterArg, aggregateCall.getCollation(),
+  aggregateCall.getType(), aggregateCall.getName()));
+}
+return newAggregateCall;
+  }
+
+  /** Transforms the condition according to the limit and offset. */
+  public static RexNode transformCondition(RexNode condition,
+  RexBuilder rexBuilder, int limit, int offset) {
+RexShuttle shuttle = new RexShuttle() {
+  @Override public RexNode visitInputRef(RexInputRef inputRef) {
+int index = inputRef.getIndex();
+if (index >= limit) {
+  return rexBuilder.makeInputRef(inputRef.getType(), index - offset);
+}
+return inputRef;
+  }
+};
+return condition.accept(shuttle);
+  }
+
+  /**
+   * Whether it can remove join according to the aggregate.
+   *
+   * @param aggregate  Aggregate
+   * @param lowerLimit lower limit (included)
+   * @param upperLimit upper limit (excluded)
 
 Review comment:
   Got it. Will explain it clearer.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

chunweilei commented on a change in pull request #1160: [CALCITE-2712] Add rule 
to eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276887188
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -735,6 +737,95 @@ public static RelNode createCastRel(
 }
   }
 
+  /** Transforms the aggregate call according to the limit and offset. */
+  public static List transformAggregateCall(
+  List aggregateCalls, int limit, int offset) {
+// Adjust arguments of the aggregate call.
+final List newAggregateCall = new ArrayList<>();
+for (AggregateCall aggregateCall : aggregateCalls) {
+  final List args = new ArrayList<>();
+  aggregateCall.getArgList()
+  .forEach(i -> args.add(i >= limit ? i - offset : i));
+
+  newAggregateCall.add(AggregateCall
+  .create(aggregateCall.getAggregation(), aggregateCall.isDistinct(),
+  aggregateCall.isApproximate(), aggregateCall.ignoreNulls(), args,
+  aggregateCall.filterArg, aggregateCall.getCollation(),
+  aggregateCall.getType(), aggregateCall.getName()));
+}
+return newAggregateCall;
+  }
+
+  /** Transforms the condition according to the limit and offset. */
+  public static RexNode transformCondition(RexNode condition,
+  RexBuilder rexBuilder, int limit, int offset) {
+RexShuttle shuttle = new RexShuttle() {
+  @Override public RexNode visitInputRef(RexInputRef inputRef) {
+int index = inputRef.getIndex();
+if (index >= limit) {
+  return rexBuilder.makeInputRef(inputRef.getType(), index - offset);
+}
+return inputRef;
+  }
+};
+return condition.accept(shuttle);
+  }
+
+  /**
+   * Whether it can remove join according to the aggregate.
+   *
+   * @param aggregate  Aggregate
+   * @param lowerLimit lower limit (included)
+   * @param upperLimit upper limit (excluded)
+   * @return Whether it can remove join or not
+   */
+  public static boolean removeJoin(Aggregate aggregate, int lowerLimit,
 
 Review comment:
   Yup.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276663148
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   hi @pengzhiwei2018 I've tried it and still got the same error.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[calcite] branch master updated: [CALCITE-2865] FilterProjectTransposeRule generates wrong traitSet when copyFilter/Project is true (Ruben Quesada Lopez)

[zabetak]

This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new 8982925  [CALCITE-2865] FilterProjectTransposeRule generates wrong 
traitSet when copyFilter/Project is true (Ruben Quesada Lopez)
8982925 is described below

commit 89829252689477bfeb305071285edcde1b4a3048
Author: rubenada 
AuthorDate: Fri Mar 1 09:50:24 2019 +0100

[CALCITE-2865] FilterProjectTransposeRule generates wrong traitSet when 
copyFilter/Project is true (Ruben Quesada Lopez)
---
 .../rel/rules/FilterProjectTransposeRule.java  | 15 +++--
 .../org/apache/calcite/test/RelOptRulesTest.java   | 38 ++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
 
b/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
index 4dc14dd..3592375 100644
--- 
a/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
+++ 
b/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
@@ -21,6 +21,9 @@ import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptRuleOperand;
 import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelDistributionTraitDef;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Project;
@@ -35,6 +38,7 @@ import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
 import org.apache.calcite.util.Util;
 
+import java.util.Collections;
 import java.util.function.Predicate;
 
 /**
@@ -155,8 +159,15 @@ public class FilterProjectTransposeRule extends RelOptRule 
{
 final RelBuilder relBuilder = call.builder();
 RelNode newFilterRel;
 if (copyFilter) {
-  newFilterRel = filter.copy(filter.getTraitSet(), project.getInput(),
-  simplifyFilterCondition(newCondition, call));
+  final RelNode input = project.getInput();
+  final RelTraitSet traitSet = filter.getTraitSet()
+  .replaceIfs(RelCollationTraitDef.INSTANCE,
+  () -> Collections.singletonList(
+  
input.getTraitSet().getTrait(RelCollationTraitDef.INSTANCE)))
+  .replaceIfs(RelDistributionTraitDef.INSTANCE,
+  () -> Collections.singletonList(
+  
input.getTraitSet().getTrait(RelDistributionTraitDef.INSTANCE)));
+  newFilterRel = filter.copy(traitSet, input, 
simplifyFilterCondition(newCondition, call));
 } else {
   newFilterRel =
   relBuilder.push(project.getInput()).filter(newCondition).build();
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java 
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 5c7a18e..1885d9d 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -29,6 +29,7 @@ import org.apache.calcite.plan.hep.HepPlanner;
 import org.apache.calcite.plan.hep.HepProgram;
 import org.apache.calcite.plan.hep.HepProgramBuilder;
 import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.RelCollation;
 import org.apache.calcite.rel.RelCollationTraitDef;
 import org.apache.calcite.rel.RelCollations;
 import org.apache.calcite.rel.RelDistributions;
@@ -135,6 +136,7 @@ import org.apache.calcite.util.ImmutableBitSet;
 
 import com.google.common.collect.ImmutableList;
 
+import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -256,6 +258,42 @@ public class RelOptRulesTest extends RelOptTestBase {
 .check();
   }
 
+  /** Test case for
+   * https://issues.apache.org/jira/browse/CALCITE-2865";>[CALCITE-2865]
+   * FilterProjectTransposeRule generates wrong traitSet when 
copyFilter/Project is true. */
+  @Test public void testFilterProjectTransposeRule() {
+List rules = Arrays.asList(
+FilterProjectTransposeRule.INSTANCE, // default: copyFilter=true, 
copyProject=true
+new FilterProjectTransposeRule(Filter.class, Project.class,
+false, false, RelFactories.LOGICAL_BUILDER));
+
+for (RelOptRule rule : rules) {
+  RelBuilder b = RelBuilder.create(RelBuilderTest.config().build());
+  RelNode in = b
+  .scan("EMP")
+  .sort(-4) // salary desc
+  .project(b.field(3)) // salary
+  .filter(b.equals(b.field(0), b.literal(11500))) // salary = 11500
+  .build();
+  HepProgram program = new HepProgramBuilder()
+  .addRuleInstance(rule)
+  .build();
+  HepPlanner hepPlanner = n

[GitHub] [calcite] zabetak merged pull request #1130: [CALCITE-2865] FilterProjectTransposeRule generates wrong traitSet when copyFilter/Project is true

[GitBox]

zabetak merged pull request #1130: [CALCITE-2865] FilterProjectTransposeRule 
generates wrong traitSet when copyFilter/Project is true   
URL: https://github.com/apache/calcite/pull/1130
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276744020
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -735,6 +737,95 @@ public static RelNode createCastRel(
 }
   }
 
+  /** Transforms the aggregate call according to the limit and offset. */
+  public static List transformAggregateCall(
+  List aggregateCalls, int limit, int offset) {
+// Adjust arguments of the aggregate call.
+final List newAggregateCall = new ArrayList<>();
+for (AggregateCall aggregateCall : aggregateCalls) {
+  final List args = new ArrayList<>();
+  aggregateCall.getArgList()
+  .forEach(i -> args.add(i >= limit ? i - offset : i));
+
+  newAggregateCall.add(AggregateCall
+  .create(aggregateCall.getAggregation(), aggregateCall.isDistinct(),
+  aggregateCall.isApproximate(), aggregateCall.ignoreNulls(), args,
+  aggregateCall.filterArg, aggregateCall.getCollation(),
+  aggregateCall.getType(), aggregateCall.getName()));
+}
+return newAggregateCall;
+  }
+
+  /** Transforms the condition according to the limit and offset. */
+  public static RexNode transformCondition(RexNode condition,
+  RexBuilder rexBuilder, int limit, int offset) {
+RexShuttle shuttle = new RexShuttle() {
+  @Override public RexNode visitInputRef(RexInputRef inputRef) {
+int index = inputRef.getIndex();
+if (index >= limit) {
+  return rexBuilder.makeInputRef(inputRef.getType(), index - offset);
+}
+return inputRef;
+  }
+};
+return condition.accept(shuttle);
+  }
+
+  /**
+   * Whether it can remove join according to the aggregate.
+   *
+   * @param aggregate  Aggregate
+   * @param lowerLimit lower limit (included)
+   * @param upperLimit upper limit (excluded)
 
 Review comment:
   The explanation of lower/upper limit is unclear.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276747445
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Project}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class ProjectJoinJoinRemoveRule extends RelOptRule {
+  public static final ProjectJoinJoinRemoveRule INSTANCE
+  = new ProjectJoinJoinRemoveRule(LogicalProject.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates a ProjectJoinJoinRemoveRule. */
+  public ProjectJoinJoinRemoveRule(
+  Class projectClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(projectClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Project project = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = 
bottomJoin.getLeft().getRowType().getFieldCount();
+
+// Check whether the project uses unexpected columns.
+if (!RelOptUtil.removeJoin(project, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
+  return;
+}
+
+// Check whether left join keys in top join and bottom join are equal.
+final List leftChildKeys = new ArrayList<>();
+final List rightChildKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(bottomJoin.getLeft(), bottomJoin.getRight(),
+bottomJoin.getCondition(), leftChildKeys, rightChildKeys,
+new ArrayList<>());
+if (!leftKeys.equals(leftChildKeys)) {
+  return;
+}
+
+// Make sure that right keys of bottom join are unique.
+final ImmutableBitSet.Builder columns = ImmutableBitSet.builder();
+rightChildKeys.forEach(key -> columns.set(key));
+final RelMetadataQuery mq = call.getMetadataQuery();
+if (!mq.areColumnsUnique(bottomJoin.getRight(), columns.build())) {
+

[GitHub] [calcite] hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276745034
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Aggregate}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class AggregateJoinJoinRemoveRule extends RelOptRule {
+  public static final AggregateJoinJoinRemoveRule INSTANCE
+  = new AggregateJoinJoinRemoveRule(LogicalAggregate.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates an AggregateJoinJoinRemoveRule. */
+  public AggregateJoinJoinRemoveRule(
+  Class aggregateClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(aggregateClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Aggregate aggregate = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = bottomJoin.getLeft().getRowType()
+.getFieldCount();
+
+// Check whether the aggregate uses unexpected columns.
+if (!RelOptUtil.removeJoin(aggregate, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
+  return;
+}
+
+// Check whether left join keys in top join and bottom join are equal.
+final List leftChildKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(bottomJoin.getLeft(), bottomJoin.getRight(),
+bottomJoin.getCondition(), leftChildKeys, new ArrayList<>(),
+new ArrayList<>());
+if (!leftKeys.equals(leftChildKeys)) {
+  return;
+}
+
+// Adjust group keys according to offset.
+int offset = bottomJoin.getRight().getRowType().getFieldCount();
+final ImmutableBitSet.Builder groupKey = ImmutableBitSet.builder();
+aggregate.getGroupSet().forEach(
+i -> groupKey.set(i >= leftBottomChildSize ? i - offset : i));
+
+final RexBuilder rexBuilder = topJoin.getCluster().getRexBuilder();
+final RelBuilder relBuilder = call.builder();
+
+relBuilder.push(bottomJoin.getLeft()).push(topJoin.getRight())
+.join(topJoin.getJoinType(),

[GitHub] [calcite] hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276744222
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -735,6 +737,95 @@ public static RelNode createCastRel(
 }
   }
 
+  /** Transforms the aggregate call according to the limit and offset. */
+  public static List transformAggregateCall(
+  List aggregateCalls, int limit, int offset) {
+// Adjust arguments of the aggregate call.
+final List newAggregateCall = new ArrayList<>();
+for (AggregateCall aggregateCall : aggregateCalls) {
+  final List args = new ArrayList<>();
+  aggregateCall.getArgList()
+  .forEach(i -> args.add(i >= limit ? i - offset : i));
+
+  newAggregateCall.add(AggregateCall
+  .create(aggregateCall.getAggregation(), aggregateCall.isDistinct(),
+  aggregateCall.isApproximate(), aggregateCall.ignoreNulls(), args,
+  aggregateCall.filterArg, aggregateCall.getCollation(),
+  aggregateCall.getType(), aggregateCall.getName()));
+}
+return newAggregateCall;
+  }
+
+  /** Transforms the condition according to the limit and offset. */
+  public static RexNode transformCondition(RexNode condition,
+  RexBuilder rexBuilder, int limit, int offset) {
+RexShuttle shuttle = new RexShuttle() {
+  @Override public RexNode visitInputRef(RexInputRef inputRef) {
+int index = inputRef.getIndex();
+if (index >= limit) {
+  return rexBuilder.makeInputRef(inputRef.getType(), index - offset);
+}
+return inputRef;
+  }
+};
+return condition.accept(shuttle);
+  }
+
+  /**
+   * Whether it can remove join according to the aggregate.
+   *
+   * @param aggregate  Aggregate
+   * @param lowerLimit lower limit (included)
+   * @param upperLimit upper limit (excluded)
+   * @return Whether it can remove join or not
+   */
+  public static boolean removeJoin(Aggregate aggregate, int lowerLimit,
 
 Review comment:
   Better call it canRemoveJoin?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276745863
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Project}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class ProjectJoinJoinRemoveRule extends RelOptRule {
+  public static final ProjectJoinJoinRemoveRule INSTANCE
+  = new ProjectJoinJoinRemoveRule(LogicalProject.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates a ProjectJoinJoinRemoveRule. */
+  public ProjectJoinJoinRemoveRule(
+  Class projectClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(projectClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Project project = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = 
bottomJoin.getLeft().getRowType().getFieldCount();
+
+// Check whether the project uses unexpected columns.
+if (!RelOptUtil.removeJoin(project, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
+  return;
+}
+
+// Check whether left join keys in top join and bottom join are equal.
+final List leftChildKeys = new ArrayList<>();
+final List rightChildKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(bottomJoin.getLeft(), bottomJoin.getRight(),
+bottomJoin.getCondition(), leftChildKeys, rightChildKeys,
+new ArrayList<>());
+if (!leftKeys.equals(leftChildKeys)) {
+  return;
+}
+
+// Make sure that right keys of bottom join are unique.
+final ImmutableBitSet.Builder columns = ImmutableBitSet.builder();
+rightChildKeys.forEach(key -> columns.set(key));
+final RelMetadataQuery mq = call.getMetadataQuery();
+if (!mq.areColumnsUnique(bottomJoin.getRight(), columns.build())) {
+

[GitHub] [calcite] hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to eliminate unnecessary outer join

[GitBox]

hsyuan commented on a change in pull request #1160: [CALCITE-2712] Add rule to 
eliminate unnecessary outer join
URL: https://github.com/apache/calcite/pull/1160#discussion_r276743099
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinJoinRemoveRule.java
 ##
 @@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that matches an {@link org.apache.calcite.rel.core.Aggregate}
+ * on a {@link org.apache.calcite.rel.core.Join} and removes the left input
+ * of the join provided that the left input is also a left join if possible.
+ *
+ * For instance,
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product as p
+ * on s.product_id = p.product_id
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ * becomes
+ *
+ * 
+ * select distinct s.product_id, pc.product_id from
+ * sales as s
+ * left join product_class pc
+ * on s.product_id = pc.product_id
+ *
+ */
+public class AggregateJoinJoinRemoveRule extends RelOptRule {
+  public static final AggregateJoinJoinRemoveRule INSTANCE
+  = new AggregateJoinJoinRemoveRule(LogicalAggregate.class,
+  LogicalJoin.class, RelFactories.LOGICAL_BUILDER);
+
+  /** Creates an AggregateJoinJoinRemoveRule. */
+  public AggregateJoinJoinRemoveRule(
+  Class aggregateClass,
+  Class joinClass, RelBuilderFactory relBuilderFactory) {
+super(
+operand(aggregateClass,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT,
+operandJ(joinClass, null,
+join -> join.getJoinType() == JoinRelType.LEFT, any(,
+relBuilderFactory, null);
+  }
+
+  @Override public void onMatch(RelOptRuleCall call) {
+final Aggregate aggregate = call.rel(0);
+final Join topJoin = call.rel(1);
+final Join bottomJoin = call.rel(2);
+int leftBottomChildSize = bottomJoin.getLeft().getRowType()
+.getFieldCount();
+
+// Check whether the aggregate uses unexpected columns.
+if (!RelOptUtil.removeJoin(aggregate, leftBottomChildSize,
+bottomJoin.getRowType().getFieldCount())) {
+  return;
+}
+
+// Check whether the top join uses unexpected columns.
+final List leftKeys = new ArrayList<>();
+RelOptUtil.splitJoinCondition(topJoin.getLeft(), topJoin.getRight(),
+topJoin.getCondition(), leftKeys, new ArrayList<>(),
+new ArrayList<>());
+if (leftKeys.stream().filter(s -> s >= leftBottomChildSize).count() > 0) {
 
 Review comment:
   I hope the if condition part can be splitted into 2 statements.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276736102
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/logical/LogicalRepeatUnion.java
 ##
 @@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.logical;
+
+import org.apache.calcite.linq4j.function.Experimental;
+import org.apache.calcite.plan.Convention;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.RepeatUnion;
+
+import java.util.List;
+
+/**
+ * Sub-class of {@link org.apache.calcite.rel.core.RepeatUnion}
+ * not targeted at any particular engine or calling convention.
+ *
+ * NOTE: The current API is experimental and subject to change without 
notice.
+ */
+@Experimental
+public class LogicalRepeatUnion extends RepeatUnion {
+
+  //~ Constructors ---
+  private LogicalRepeatUnion(
+int maxRep,
+RelOptCluster cluster,
+RelTraitSet traitSet,
+RelNode seed,
+RelNode iterative) {
+super(maxRep, cluster, traitSet, seed, iterative);
+  }
+
+  /** Creates a LogicalRepeatUnion. */
+  public static LogicalRepeatUnion create(RelNode seed, RelNode iterative) {
+return create(seed, iterative, -1);
+  }
+
+  /** Creates a LogicalRepeatUnion. */
+  public static LogicalRepeatUnion create(RelNode seed, RelNode iterative, int 
maxRep) {
+RelOptCluster cluster = seed.getCluster();
+RelTraitSet traitSet = cluster.traitSetOf(Convention.NONE);
+return new LogicalRepeatUnion(maxRep, cluster, traitSet, seed, iterative);
 
 Review comment:
   I followed here the same logic as LogicalUnion, I'm not sure we can / should 
do otherwise


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276735774
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/schema/impl/TransientTable.java
 ##
 @@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.schema.impl;
+
+import org.apache.calcite.DataContext;
+import org.apache.calcite.adapter.java.AbstractQueryableTable;
+import org.apache.calcite.linq4j.AbstractEnumerable;
+import org.apache.calcite.linq4j.Enumerable;
+import org.apache.calcite.linq4j.Enumerator;
+import org.apache.calcite.linq4j.Linq4j;
+import org.apache.calcite.linq4j.QueryProvider;
+import org.apache.calcite.linq4j.Queryable;
+import org.apache.calcite.linq4j.function.Experimental;
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.TableModify;
+import org.apache.calcite.rel.logical.LogicalTableModify;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.schema.ModifiableTable;
+import org.apache.calcite.schema.ScannableTable;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Schemas;
+
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+/**
+ * Simple modifiable table backed by a Java list.
 
 Review comment:
   Class renamed, Javadoc updated.
   extend AbstractQueryableTable is needed because interface ModifiableTable 
extends QueryableTable


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276735138
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Spool.java
 ##
 @@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.core;
+
+import org.apache.calcite.linq4j.function.Experimental;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.SingleRel;
+
+import java.util.List;
+
+/**
+ * Relational expression that consumes a set of data, and stores it in a 
temporary structure.
+ *
+ * NOTE: The current API is experimental and subject to change without 
notice.
+ */
+@Experimental
+public abstract class Spool extends SingleRel {
+
+  /**
+   * Enumeration representing spool read / write type (all four combinations 
are possible):
 
 Review comment:
   Javadoc updated
   I think that readType/writeType are more meaningful (and clear) than 
iterateType/forwardType


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276734751
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Spool.java
 ##
 @@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.core;
+
+import org.apache.calcite.linq4j.function.Experimental;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.SingleRel;
+
+import java.util.List;
+
+/**
+ * Relational expression that consumes a set of data, and stores it in a 
temporary structure.
 
 Review comment:
   Comment modified


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276734115
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableSpool.java
 ##
 @@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.linq4j.function.Experimental;
+import org.apache.calcite.linq4j.tree.BlockBuilder;
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.linq4j.tree.Expressions;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.Spool;
+import org.apache.calcite.schema.ModifiableTable;
+import org.apache.calcite.util.BuiltInMethod;
+
+/**
+ * Implementation of {@link Spool} in
+ * {@link EnumerableConvention enumerable calling convention}
+ * that writes into a {@link ModifiableTable}
+ *
+ * NOTE: The current API is experimental and subject to change without 
notice.
+ */
+@Experimental
+public class EnumerableTableSpool extends Spool implements EnumerableRel {
+
+  // table name, it must refer to an existing ModifiableTable
 
 Review comment:
   Comment updated


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] vineetgarg02 commented on a change in pull request #1172: [CALCITE-3007] Type mismatch for ANY subquery in project (Vineet Garg)

[GitBox]

vineetgarg02 commented on a change in pull request #1172: [CALCITE-3007] Type 
mismatch for ANY subquery in project (Vineet Garg)
URL: https://github.com/apache/calcite/pull/1172#discussion_r276717705
 
 

 ##
 File path: core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
 ##
 @@ -493,12 +493,86 @@ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], 
MGR=[$3], HIREDATE=[$4], SAL=[$
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

[GitHub] [calcite] nishantmonu51 commented on a change in pull request #1174: [CALCITE-3001] Upgrade to Apache Druid 0.14.0-incubating

[GitBox]

nishantmonu51 commented on a change in pull request #1174: [CALCITE-3001] 
Upgrade to Apache Druid 0.14.0-incubating
URL: https://github.com/apache/calcite/pull/1174#discussion_r276709067
 
 

 ##
 File path: druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
 ##
 @@ -3058,15 +3058,15 @@ private void testCountWithApproxDistinct(boolean 
approx, String sql, String expe
 + "(count(distinct \"user_id\") * 2) from \"wiki\"";
 wikiApprox(sql)
 .queryContains(druidChecker(druid))
-.returnsUnordered("EXPR$0=-10590");
+.returnsUnordered("EXPR$0=100");
 
 Review comment:
   will look into it and verify 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:
us...@infra.apache.org


With regards,
Apache Git Services

[calcite] branch master updated: [CALCITE-3006] Example code on site cannot compile (Chunwei Lei)

[hyuan]

This is an automated email from the ASF dual-hosted git repository.

hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new 41df09c  [CALCITE-3006] Example code on site cannot compile (Chunwei 
Lei)
41df09c is described below

commit 41df09cb3a842b43606fa69bbcb25a70d8250bdb
Author: Chunwei Lei 
AuthorDate: Thu Apr 18 11:19:46 2019 +0800

[CALCITE-3006] Example code on site cannot compile (Chunwei Lei)
---
 site/_docs/index.md | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/site/_docs/index.md b/site/_docs/index.md
index bbff998..cf4560c 100644
--- a/site/_docs/index.md
+++ b/site/_docs/index.md
@@ -50,8 +50,7 @@ Connection connection =
 CalciteConnection calciteConnection =
 connection.unwrap(CalciteConnection.class);
 SchemaPlus rootSchema = calciteConnection.getRootSchema();
-Schema schema = ReflectiveSchema.create(calciteConnection,
-rootSchema, "hr", new HrSchema());
+Schema schema = new ReflectiveSchema(new HrSchema());
 rootSchema.add("hr", schema);
 Statement statement = calciteConnection.createStatement();
 ResultSet resultSet = statement.executeQuery(
@@ -68,7 +67,7 @@ connection.close();
 {% endhighlight %}
 
 Where is the database? There is no database. The connection is
-completely empty until `ReflectiveSchema.create` registers a Java
+completely empty until `new ReflectiveSchema` registers a Java
 object as a schema and its collection fields `emps` and `depts` as
 tables.
 
@@ -79,8 +78,7 @@ library. But Calcite can also process data in other data 
formats, such
 as JDBC. In the first example, replace
 
 {% highlight java %}
-Schema schema = ReflectiveSchema.create(calciteConnection,
-rootSchema, "hr", new HrSchema());
+Schema schema = new ReflectiveSchema(new HrSchema());
 {% endhighlight %}
 
 with

[GitHub] [calcite] hsyuan merged pull request #1173: [CALCITE-3006] Example code on site cannot compile

[GitBox]

hsyuan merged pull request #1173: [CALCITE-3006] Example code on site cannot 
compile
URL: https://github.com/apache/calcite/pull/1173
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] jcamachor commented on a change in pull request #1174: [CALCITE-3001] Upgrade to Apache Druid 0.14.0-incubating

[GitBox]

jcamachor commented on a change in pull request #1174: [CALCITE-3001] Upgrade 
to Apache Druid 0.14.0-incubating
URL: https://github.com/apache/calcite/pull/1174#discussion_r276703659
 
 

 ##
 File path: druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
 ##
 @@ -3058,15 +3058,15 @@ private void testCountWithApproxDistinct(boolean 
approx, String sql, String expe
 + "(count(distinct \"user_id\") * 2) from \"wiki\"";
 wikiApprox(sql)
 .queryContains(druidChecker(druid))
-.returnsUnordered("EXPR$0=-10590");
+.returnsUnordered("EXPR$0=100");
 
 // Change COUNT(DISTINCT ...) to APPROX_COUNT_DISTINCT(...) and get
 // same result even if approximation is off by default.
 final String sql2 = "select (approx_count_distinct(\"user_id\") + 100) - "
 + "(approx_count_distinct(\"user_id\") * 2) from \"wiki\"";
 sql(sql2, WIKI)
 .queryContains(druidChecker(druid))
-.returnsUnordered("EXPR$0=-10590");
+.returnsUnordered("EXPR$0=100");
 
 Review comment:
   Is this change of result expected (same as above)?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] jcamachor commented on a change in pull request #1174: [CALCITE-3001] Upgrade to Apache Druid 0.14.0-incubating

[GitBox]

jcamachor commented on a change in pull request #1174: [CALCITE-3001] Upgrade 
to Apache Druid 0.14.0-incubating
URL: https://github.com/apache/calcite/pull/1174#discussion_r276703539
 
 

 ##
 File path: druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
 ##
 @@ -3058,15 +3058,15 @@ private void testCountWithApproxDistinct(boolean 
approx, String sql, String expe
 + "(count(distinct \"user_id\") * 2) from \"wiki\"";
 wikiApprox(sql)
 .queryContains(druidChecker(druid))
-.returnsUnordered("EXPR$0=-10590");
+.returnsUnordered("EXPR$0=100");
 
 Review comment:
   Is this change of result expected?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] chunweilei commented on issue #1167: [CALCITE-3009] It should fail if there are duplicate keys in a .xml file

[GitBox]

chunweilei commented on issue #1167: [CALCITE-3009] It should fail if there are 
duplicate keys in a .xml file
URL: https://github.com/apache/calcite/pull/1167#issuecomment-484537960
 
 
   @julianhyde could you please review this PR? Appreciate 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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276663148
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   hi @pengzhiwei2018 I've tried it and still got the same error.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276568835
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRepeatUnionRule.java
 ##
 @@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.plan.Convention;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.convert.ConverterRule;
+import org.apache.calcite.rel.logical.LogicalRepeatUnion;
+
+/**
+ * Rule to convert a {@link LogicalRepeatUnion} into an {@link 
EnumerableRepeatUnion}.
+ */
+public class EnumerableRepeatUnionRule extends ConverterRule {
+
+  public EnumerableRepeatUnionRule() {
+super(
+  LogicalRepeatUnion.class,
+  Convention.NONE,
+  EnumerableConvention.INSTANCE,
+  "EnumerableRepeatUnionRule");
+
+  }
+
+  @Override public RelNode convert(RelNode rel) {
+LogicalRepeatUnion union = (LogicalRepeatUnion) rel;
+EnumerableConvention out = EnumerableConvention.INSTANCE;
+RelTraitSet traitSet = union.getTraitSet().replace(out);
+RelNode seedRel = union.getSeedRel();
+RelNode iterativeRel = union.getIterativeRel();
+
+return new EnumerableRepeatUnion(
 
 Review comment:
   This is not the case for other similar operators (EnumerableUnion / Minus / 
Intersect), that EnumerableRepeatUnion is "inspired" by. I guess because 
RelTraitSet must be deduced from the LogicalUnion / Minus / Intersect at rule 
matching time?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] nishantmonu51 opened a new pull request #1174: [CALCITE-3001] Upgrade to Apache Druid 0.14.0-incubating

[GitBox]

nishantmonu51 opened a new pull request #1174: [CALCITE-3001] Upgrade to Apache 
Druid 0.14.0-incubating
URL: https://github.com/apache/calcite/pull/1174
 
 
   Depends on https://github.com/vlsi/calcite-test-dataset/pull/30


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] pengzhiwei2018 commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

pengzhiwei2018 commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276625244
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   Hi @XuQianJin-Stars ,you can define a `LeftFunctionCall()`  in 
`BuiltinFunctionCall()` at the Parser.jj, just like the 
`TimestampAddFunctionCall()` does.  It works well for me .May you success.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] pengzhiwei2018 commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

pengzhiwei2018 commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276625244
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   Hi @XuQianJin-Stars ,you can define a `LeftFunctionCall()`  in 
`BuiltinFunctionCall()` at the Parser.jj, just like the 
`TimestampAddFunctionCall()` does.  It works well for me .May you succeed.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] pengzhiwei2018 commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

pengzhiwei2018 commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276625244
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   Hi @XuQianJin-Stars ,you can define a LeftFunctionCall in 
`BuiltinFunctionCall()` of the Parser.jj, just like the 
`TimestampAddFunctionCall()` does.  It works well for me .May you succeed.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

XuQianJin-Stars commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276611604
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   hi @julianhyde Thank you very much.It looks like there's a conflict between 
String's LEFT semantics and LEFT JOIN semantics.I am not sure how to solve this 
problem. Could you give me some advice?
   best
   qianjin


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276601185
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   There are 2 rules `ReduceExpressionsRule` and `FilterJoinRule` that use this 
method to make some decision when planning, this method returns what the 
original EquiJoin is, cause now the join.analyzeCondition().isEqui() has wider 
scope that this method returns. I thought maybe we should move the logic to the 
rules or tweak the rules logic.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276601185
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   There are 2 rules that use this method to make some decision when planning, 
this method returns what the original EquiJoin is, cause now the 
join.analyzeCondition().isEqui() has wider scope that this method returns. I 
thought maybe we should move the logic to the rules or tweak the rules logic.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276599667
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   In the beginning, i add this method to distinguish the `SemiJoin` 
transformed from `Correlate` and `SemiJoin` from `SemiJoinRule` or 
`RelBuilder`. Cause i thought that the join comes from a `Correlate` can make a 
JoinRelType.SEMI join type.
   
   But in current code, Calcite will never transform from Correlate to join 
with `JoinRelType.SEMI`(only left and inner join type), so this method actually 
can simplify to just decide if the join type is `JoinRelType.SEMI`.
   
   But i still keep the variables check and i think correlate should have the 
ability to get a semi join type join.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276591409
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableNestedLoopJoin.java
 ##
 @@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.linq4j.tree.BlockBuilder;
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.linq4j.tree.Expressions;
+import org.apache.calcite.linq4j.tree.ParameterExpression;
+import org.apache.calcite.linq4j.tree.Primitive;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.BuiltInMethod;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Type;
+import java.util.Set;
+
+/** Implementation of {@link org.apache.calcite.rel.core.Correlate} in
+ * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}. */
+public class EnumerableNestedLoopJoin extends Join
+implements EnumerableRel {
+  private final ImmutableBitSet requiredColumns;
+
+  public EnumerableNestedLoopJoin(RelOptCluster cluster, RelTraitSet traits,
+  RelNode left, RelNode right, RexNode condition, Set 
variablesSet,
+  ImmutableBitSet requiredColumns, JoinRelType joinType) {
+super(cluster, traits, left, right, condition, variablesSet, joinType);
+this.requiredColumns = requiredColumns;
+  }
+
+  /** Creates an EnumerableNestedLoopJoin. */
+  public static EnumerableNestedLoopJoin create(
+  RelNode left,
+  RelNode right,
+  RexNode condition,
+  Set variablesSet,
+  ImmutableBitSet requiredColumns,
+  JoinRelType joinType) {
+assert variablesSet.size() == 0 || variablesSet.size() == 1;
+final RelOptCluster cluster = left.getCluster();
+final RelMetadataQuery mq = cluster.getMetadataQuery();
+final RelTraitSet traitSet =
+cluster.traitSetOf(EnumerableConvention.INSTANCE)
+.replaceIfs(RelCollationTraitDef.INSTANCE,
+() -> RelMdCollation.enumerableCorrelate(mq, left, right, 
joinType));
+return new EnumerableNestedLoopJoin(
+cluster,
+traitSet,
+left,
+right,
+condition,
+variablesSet,
+requiredColumns,
+joinType);
+  }
+
+  @Override public RelOptCost computeSelfCost(RelOptPlanner planner,
+  RelMetadataQuery mq) {
+double rowCount = mq.getRowCount(this);
+
+// Right-hand input is the "build", and hopefully small, input.
+final double rightRowCount = right.estimateRowCount(mq);
+final double leftRowCount = left.estimateRowCount(mq);
+
+if (isNonCorrelateSemiJoin()) {
+  if (Double.isInfinite(leftRowCount)) {
+rowCount = leftRowCount;
+  } else {
+rowCount += Util.nLogN(leftRowCount);
+  }
+  if (Double.isInfinite(rightRowCount)) {
+rowCount = rightRowCount;
+  } else {
+rowCount += rightRowCount;
+  }
+  return planner.getCostFactory().makeCost(rowCount, 0, 
0).multiplyBy(.01d);
+} else {
+  if (Double.isInfinite(leftRowCount) || Double.isInfinite(rightRowCount)) 
{
+return planner.getCostFactory().makeInfiniteCost();
+  }
+
+  Double restartCount = mq.getRowCount(getLeft());
+  // RelMetadataQuery.getCumulativeCost(getRight()); does not work for
+  // RelSubset, so we ask pl

[GitHub] [calcite] danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

danny0405 commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276591409
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableNestedLoopJoin.java
 ##
 @@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.linq4j.tree.BlockBuilder;
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.linq4j.tree.Expressions;
+import org.apache.calcite.linq4j.tree.ParameterExpression;
+import org.apache.calcite.linq4j.tree.Primitive;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.BuiltInMethod;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Type;
+import java.util.Set;
+
+/** Implementation of {@link org.apache.calcite.rel.core.Correlate} in
+ * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}. */
+public class EnumerableNestedLoopJoin extends Join
+implements EnumerableRel {
+  private final ImmutableBitSet requiredColumns;
+
+  public EnumerableNestedLoopJoin(RelOptCluster cluster, RelTraitSet traits,
+  RelNode left, RelNode right, RexNode condition, Set 
variablesSet,
+  ImmutableBitSet requiredColumns, JoinRelType joinType) {
+super(cluster, traits, left, right, condition, variablesSet, joinType);
+this.requiredColumns = requiredColumns;
+  }
+
+  /** Creates an EnumerableNestedLoopJoin. */
+  public static EnumerableNestedLoopJoin create(
+  RelNode left,
+  RelNode right,
+  RexNode condition,
+  Set variablesSet,
+  ImmutableBitSet requiredColumns,
+  JoinRelType joinType) {
+assert variablesSet.size() == 0 || variablesSet.size() == 1;
+final RelOptCluster cluster = left.getCluster();
+final RelMetadataQuery mq = cluster.getMetadataQuery();
+final RelTraitSet traitSet =
+cluster.traitSetOf(EnumerableConvention.INSTANCE)
+.replaceIfs(RelCollationTraitDef.INSTANCE,
+() -> RelMdCollation.enumerableCorrelate(mq, left, right, 
joinType));
+return new EnumerableNestedLoopJoin(
+cluster,
+traitSet,
+left,
+right,
+condition,
+variablesSet,
+requiredColumns,
+joinType);
+  }
+
+  @Override public RelOptCost computeSelfCost(RelOptPlanner planner,
+  RelMetadataQuery mq) {
+double rowCount = mq.getRowCount(this);
+
+// Right-hand input is the "build", and hopefully small, input.
+final double rightRowCount = right.estimateRowCount(mq);
+final double leftRowCount = left.estimateRowCount(mq);
+
+if (isNonCorrelateSemiJoin()) {
+  if (Double.isInfinite(leftRowCount)) {
+rowCount = leftRowCount;
+  } else {
+rowCount += Util.nLogN(leftRowCount);
+  }
+  if (Double.isInfinite(rightRowCount)) {
+rowCount = rightRowCount;
+  } else {
+rowCount += rightRowCount;
+  }
+  return planner.getCostFactory().makeCost(rowCount, 0, 
0).multiplyBy(.01d);
+} else {
+  if (Double.isInfinite(leftRowCount) || Double.isInfinite(rightRowCount)) 
{
+return planner.getCostFactory().makeInfiniteCost();
+  }
+
+  Double restartCount = mq.getRowCount(getLeft());
+  // RelMetadataQuery.getCumulativeCost(getRight()); does not work for
+  // RelSubset, so we ask pl

[GitHub] [calcite] julianhyde commented on a change in pull request #1168: [CALCITE-3005] Implement string functions: LEFT, RIGHT

[GitBox]

julianhyde commented on a change in pull request #1168: [CALCITE-3005] 
Implement string functions: LEFT, RIGHT
URL: https://github.com/apache/calcite/pull/1168#discussion_r276577645
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
 ##
 @@ -1508,6 +1508,24 @@ public SqlOperandCountRange getOperandCountRange() {
   OperandTypes.CHARACTER,
   SqlFunctionCategory.STRING);
 
+  public static final SqlFunction LEFT =
+  new SqlFunction(
+  "LEFT",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.VARCHAR_2000,
 
 Review comment:
   The type inference should be the same as SUBSTRING. It can return null if 
and only if at least one argument is null.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1020: [CALCITE-2812] Add algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)

[GitBox]

rubenada commented on a change in pull request #1020: [CALCITE-2812] Add 
algebraic operators to allow expressing recursive queries (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/1020#discussion_r276568835
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRepeatUnionRule.java
 ##
 @@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.plan.Convention;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.convert.ConverterRule;
+import org.apache.calcite.rel.logical.LogicalRepeatUnion;
+
+/**
+ * Rule to convert a {@link LogicalRepeatUnion} into an {@link 
EnumerableRepeatUnion}.
+ */
+public class EnumerableRepeatUnionRule extends ConverterRule {
+
+  public EnumerableRepeatUnionRule() {
+super(
+  LogicalRepeatUnion.class,
+  Convention.NONE,
+  EnumerableConvention.INSTANCE,
+  "EnumerableRepeatUnionRule");
+
+  }
+
+  @Override public RelNode convert(RelNode rel) {
+LogicalRepeatUnion union = (LogicalRepeatUnion) rel;
+EnumerableConvention out = EnumerableConvention.INSTANCE;
+RelTraitSet traitSet = union.getTraitSet().replace(out);
+RelNode seedRel = union.getSeedRel();
+RelNode iterativeRel = union.getIterativeRel();
+
+return new EnumerableRepeatUnion(
 
 Review comment:
   This is not the case for other similar operators (EnumerableUnion / Minus / 
Intersect), that EnumerableRepeatUnion is "inspired" by. I guess because 
RelTraitSet must be deduce from the LogicalUnion / Minus / Intersect at rule 
matching time. 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276564619
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   Agree, couldn't it simply be: `return join.analyzeCondition().isEqui();`


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276564619
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   Agree, couldn't it simply be: `return join.analyzeCondition().isEqui();`  ?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276558022
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/core/Join.java
 ##
 @@ -230,6 +237,17 @@ public boolean isSemiJoinDone() {
 return false;
   }
 
+  /**
+   * Returns whether this LogicalJoin is a semi-join but does
+   * not comes from decorrelate.
+   *
+   * @return true if this is semi but without correlate variables.
+   */
+  public boolean isNonCorrelateSemiJoin() {
+return (this.variablesSet == null || this.variablesSet.size() == 0)
+&& joinType == JoinRelType.SEMI;
+  }
 
 Review comment:
   The addition of this method confuses me.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276557077
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableThetaJoin.java
 ##
 @@ -45,6 +46,7 @@
 /** Implementation of {@link org.apache.calcite.rel.core.Join} in
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}
  * that allows conditions that are not just {@code =} (equals). */
+@Deprecated // to be removed before 2.0
 
 Review comment:
   Actually, looking at BuiltInMethod.THETA_JOIN, I agree it is the pure 
definition of a NestedLoopJoin (without the correlating variable mechanism). 
So, it would seem that we two implementations of nested loop join 
(EnumerableCorrelate and EnumerableThetaJoin). What if we did the following:
   - Rename EnumerableThetaJoin as EnumerableNestedLoopJoin: right now this is 
most powerful (albeit most likely inefficient) way to implement any type of 
join: inner, left, right, outer; with equi or non-equi condition (although the 
inner and equi ones actually go through EnumerableJoin for efficiency). And I 
think EnumerableThetaJoin will allow us to implement non-equi semijoins too.
   - Keep EnumerableCorrelate as it is, do not rename it. I agree that it is 
also a nested loop implementation, but it uses the correlating variable 
approach, so I think keeping its current name will keep things clearer.
   This is an alternative approach that differs from the original idea, I'm not 
sure if it's the right one, but me be worth to discuss 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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276557077
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableThetaJoin.java
 ##
 @@ -45,6 +46,7 @@
 /** Implementation of {@link org.apache.calcite.rel.core.Join} in
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}
  * that allows conditions that are not just {@code =} (equals). */
+@Deprecated // to be removed before 2.0
 
 Review comment:
   Actually, looking at BuiltInMethod.THETA_JOIN, I agree it is the pure 
definition of a NestedLoopJoin (without the correlating variable mechanism). 
So, it would seem that we two implementations of nested loop join 
(EnumerableCorrelate and EnumerableThetaJoin). What if we did the following:
   - Rename EnumerableThetaJoin as EnumerableNestedLoopJoin: right now this is 
most powerful (albeit most likely inefficient) way to implement any type of 
join: inner, left, right, outer; with equi or non-equi condition (although the 
inner and equi ones actually go through EnumerableJoin for efficiency). And I 
think EnumerableThetaJoin will allow us to implement non-equi semijoins too.
   - Keep EnumerableCorrelate as it is, do not rename it. I agree that it is 
also a nested loop implementation, but it uses the correlating variable 
approach, so I think keeping its current name will keep things clearer.
   
   This is an alternative approach that differs from the original idea, I'm not 
sure if it's the right one, but me be worth to discuss 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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [calcite] rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

rubenada commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276553754
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableNestedLoopJoin.java
 ##
 @@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.linq4j.tree.BlockBuilder;
+import org.apache.calcite.linq4j.tree.Expression;
+import org.apache.calcite.linq4j.tree.Expressions;
+import org.apache.calcite.linq4j.tree.ParameterExpression;
+import org.apache.calcite.linq4j.tree.Primitive;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.BuiltInMethod;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Type;
+import java.util.Set;
+
+/** Implementation of {@link org.apache.calcite.rel.core.Correlate} in
+ * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}. */
+public class EnumerableNestedLoopJoin extends Join
+implements EnumerableRel {
+  private final ImmutableBitSet requiredColumns;
+
+  public EnumerableNestedLoopJoin(RelOptCluster cluster, RelTraitSet traits,
+  RelNode left, RelNode right, RexNode condition, Set 
variablesSet,
+  ImmutableBitSet requiredColumns, JoinRelType joinType) {
+super(cluster, traits, left, right, condition, variablesSet, joinType);
+this.requiredColumns = requiredColumns;
+  }
+
+  /** Creates an EnumerableNestedLoopJoin. */
+  public static EnumerableNestedLoopJoin create(
+  RelNode left,
+  RelNode right,
+  RexNode condition,
+  Set variablesSet,
+  ImmutableBitSet requiredColumns,
+  JoinRelType joinType) {
+assert variablesSet.size() == 0 || variablesSet.size() == 1;
+final RelOptCluster cluster = left.getCluster();
+final RelMetadataQuery mq = cluster.getMetadataQuery();
+final RelTraitSet traitSet =
+cluster.traitSetOf(EnumerableConvention.INSTANCE)
+.replaceIfs(RelCollationTraitDef.INSTANCE,
+() -> RelMdCollation.enumerableCorrelate(mq, left, right, 
joinType));
+return new EnumerableNestedLoopJoin(
+cluster,
+traitSet,
+left,
+right,
+condition,
+variablesSet,
+requiredColumns,
+joinType);
+  }
+
+  @Override public RelOptCost computeSelfCost(RelOptPlanner planner,
+  RelMetadataQuery mq) {
+double rowCount = mq.getRowCount(this);
+
+// Right-hand input is the "build", and hopefully small, input.
+final double rightRowCount = right.estimateRowCount(mq);
+final double leftRowCount = left.estimateRowCount(mq);
+
+if (isNonCorrelateSemiJoin()) {
+  if (Double.isInfinite(leftRowCount)) {
+rowCount = leftRowCount;
+  } else {
+rowCount += Util.nLogN(leftRowCount);
+  }
+  if (Double.isInfinite(rightRowCount)) {
+rowCount = rightRowCount;
+  } else {
+rowCount += rightRowCount;
+  }
+  return planner.getCostFactory().makeCost(rowCount, 0, 
0).multiplyBy(.01d);
+} else {
+  if (Double.isInfinite(leftRowCount) || Double.isInfinite(rightRowCount)) 
{
+return planner.getCostFactory().makeInfiniteCost();
+  }
+
+  Double restartCount = mq.getRowCount(getLeft());
+  // RelMetadataQuery.getCumulativeCost(getRight()); does not work for
+  // RelSubset, so we ask pla

[GitHub] [calcite] hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve design of join-like relational expressions

[GitBox]

hsyuan commented on a change in pull request #1157: [CALCITE-2969] Improve 
design of join-like relational expressions
URL: https://github.com/apache/calcite/pull/1157#discussion_r276553183
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
 ##
 @@ -3734,6 +3740,13 @@ private Exists(RelNode r, boolean indicator, boolean 
outerJoin) {
   this.outerJoin = outerJoin;
 }
   }
+
+  /** Check if it is the join whose condition is based on column equality. */
+  public static boolean isEquiJoin(Join join) {
+return join.isNonCorrelateSemiJoin()
+|| join instanceof EnumerableHashJoin
+|| join instanceof EnumerableMergeJoin;
+  }
 
 Review comment:
   This looks weird.


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:
us...@infra.apache.org


With regards,
Apache Git Services