You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2016/12/07 02:09:30 UTC

[1/2] calcite git commit: [CALCITE-1522] Fix error message for SetOp with incompatible args (Jess Balint)

Repository: calcite
Updated Branches:
  refs/heads/master c397f37bd -> ffe1a68d7


[CALCITE-1522] Fix error message for SetOp with incompatible args (Jess Balint)

Close apache/calcite#331


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/e4d3ad0b
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/e4d3ad0b
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/e4d3ad0b

Branch: refs/heads/master
Commit: e4d3ad0bdc3beb03e5777b82d15684c6c78e12ee
Parents: c397f37
Author: Jess Balint <jb...@gmail.com>
Authored: Thu Dec 1 14:07:21 2016 -0600
Committer: Julian Hyde <jh...@apache.org>
Committed: Tue Dec 6 16:19:13 2016 -0800

----------------------------------------------------------------------
 .../java/org/apache/calcite/rel/core/SetOp.java | 24 +++++++++++-------
 .../org/apache/calcite/tools/RelBuilder.java    |  2 +-
 .../org/apache/calcite/test/RelBuilderTest.java | 26 ++++++++++++++++++++
 3 files changed, 42 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/e4d3ad0b/core/src/main/java/org/apache/calcite/rel/core/SetOp.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/core/SetOp.java b/core/src/main/java/org/apache/calcite/rel/core/SetOp.java
index cd58192..930f23f 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/SetOp.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/SetOp.java
@@ -26,11 +26,13 @@ import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.RelWriter;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.util.Util;
 
+import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 
-import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -100,16 +102,20 @@ public abstract class SetOp extends AbstractRelNode {
   }
 
   @Override protected RelDataType deriveRowType() {
-    return getCluster().getTypeFactory().leastRestrictive(
-        new AbstractList<RelDataType>() {
-          @Override public RelDataType get(int index) {
-            return inputs.get(index).getRowType();
-          }
-
-          @Override public int size() {
-            return inputs.size();
+    final List<RelDataType> inputRowTypes = Lists.transform(inputs,
+        new Function<RelNode, RelDataType>() {
+          public RelDataType apply(RelNode input) {
+            return input.getRowType();
           }
         });
+    final RelDataType rowType =
+        getCluster().getTypeFactory().leastRestrictive(inputRowTypes);
+    if (rowType == null) {
+      throw new IllegalArgumentException("Cannot compute compatible row type "
+          + "for arguments to set op: "
+          + Util.sepList(inputRowTypes, ", "));
+    }
+    return rowType;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/calcite/blob/e4d3ad0b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 9608059..1eaf1c6 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -403,7 +403,7 @@ public class RelBuilder {
       aliases.add(pair.left);
       offset += pair.right.getFieldCount();
     }
-    throw new IllegalArgumentException("no relation wtih alias '" + alias
+    throw new IllegalArgumentException("no relation with alias '" + alias
         + "'; aliases are: " + aliases);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/e4d3ad0b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index c79fbdc..48c8d60 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -712,6 +712,32 @@ public class RelBuilderTest {
             + "      LogicalTableScan(table=[[scott, EMP]])\n"));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1522">[CALCITE-1522]
+   * Fix error message for SetOp with incompatible args</a>. */
+  @Test public void testBadUnionArgsErrorMessage() {
+    // Equivalent SQL:
+    //   SELECT EMPNO, SAL FROM emp
+    //   UNION ALL
+    //   SELECT DEPTNO FROM dept
+    final RelBuilder builder = RelBuilder.create(config().build());
+    try {
+      final RelNode root =
+          builder.scan("DEPT")
+              .project(builder.field("DEPTNO"))
+              .scan("EMP")
+              .project(builder.field("EMPNO"), builder.field("SAL"))
+              .union(true)
+              .build();
+      fail("Expected error, got " + root);
+    } catch (IllegalArgumentException e) {
+      final String expected = "Cannot compute compatible row type for "
+          + "arguments to set op: RecordType(TINYINT DEPTNO), "
+          + "RecordType(SMALLINT EMPNO, DECIMAL(7, 2) SAL)";
+      assertThat(e.getMessage(), is(expected));
+    }
+  }
+
   @Test public void testUnion3() {
     // Equivalent SQL:
     //   SELECT deptno FROM dept


[2/2] calcite git commit: [CALCITE-1523] In RelBuilder, add "field" method to reference input to join by alias (Jess Balint)

Posted by jh...@apache.org.
[CALCITE-1523] In RelBuilder, add "field" method to reference input to join by alias (Jess Balint)

Close apache/calcite#332


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/ffe1a68d
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/ffe1a68d
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/ffe1a68d

Branch: refs/heads/master
Commit: ffe1a68d7d3cc4e382f8f6ce7ce1beb02860f61a
Parents: e4d3ad0
Author: Jess Balint <jb...@gmail.com>
Authored: Thu Dec 1 14:07:21 2016 -0600
Committer: Julian Hyde <jh...@apache.org>
Committed: Tue Dec 6 16:23:23 2016 -0800

----------------------------------------------------------------------
 .../org/apache/calcite/tools/RelBuilder.java    | 36 +++++++----
 .../org/apache/calcite/test/RelBuilderTest.java | 67 ++++++++++++++++++++
 site/_docs/algebra.md                           |  7 +-
 3 files changed, 95 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/ffe1a68d/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 1eaf1c6..c70725d 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -384,24 +384,34 @@ public class RelBuilder {
   /** Creates a reference to a field of the current record which originated
    * in a relation with a given alias. */
   public RexNode field(String alias, String fieldName) {
+    return field(1, alias, fieldName);
+  }
+
+  /** Creates a reference to a field which originated in a relation with the
+   * given alias. Searches for the relation starting at the top of the
+   * stack. */
+  public RexNode field(int inputCount, String alias, String fieldName) {
     Preconditions.checkNotNull(alias);
     Preconditions.checkNotNull(fieldName);
-    final Frame frame = stack.peek();
     final List<String> aliases = new ArrayList<>();
-    int offset = 0;
-    for (Pair<String, RelDataType> pair : frame.right) {
-      if (pair.left != null && pair.left.equals(alias)) {
-        int i = pair.right.getFieldNames().indexOf(fieldName);
-        if (i >= 0) {
-          return field(offset + i);
-        } else {
-          throw new IllegalArgumentException("no field '" + fieldName
-              + "' in relation '" + alias
-              + "'; fields are: " + pair.right.getFieldNames());
+    for (int inputOrdinal = 0; inputOrdinal < inputCount; ++inputOrdinal) {
+      final Frame frame = peek_(inputOrdinal);
+      int offset = 0; // relative to this frame
+      for (Pair<String, RelDataType> pair : frame.right) {
+        if (pair.left != null && pair.left.equals(alias)) {
+          int i = pair.right.getFieldNames().indexOf(fieldName);
+          if (i >= 0) {
+            return field(inputCount, inputCount - 1 - inputOrdinal,
+                offset + i);
+          } else {
+            throw new IllegalArgumentException("no field '" + fieldName
+                + "' in relation '" + alias
+                + "'; fields are: " + pair.right.getFieldNames());
+          }
         }
+        aliases.add(pair.left);
+        offset += pair.right.getFieldCount();
       }
-      aliases.add(pair.left);
-      offset += pair.right.getFieldCount();
     }
     throw new IllegalArgumentException("no relation with alias '" + alias
         + "'; aliases are: " + aliases);

http://git-wip-us.apache.org/repos/asf/calcite/blob/ffe1a68d/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index 48c8d60..a19947a 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -1034,6 +1034,73 @@ public class RelBuilderTest {
     assertThat(str(root), is(expected));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1523">[CALCITE-1523]
+   * Add RelBuilder field() method to reference aliased relations not on top of
+   * stack</a>, accessing tables aliased that are not accessible in the top
+   * RelNode. */
+  @Test public void testAliasPastTop() {
+    // Equivalent SQL:
+    //   SELECT *
+    //   FROM emp
+    //   LEFT JOIN dept ON emp.deptno = dept.deptno
+    //     AND emp.empno = 123
+    //     AND dept.deptno IS NOT NULL
+    final RelBuilder builder = RelBuilder.create(config().build());
+    RelNode root =
+        builder.scan("EMP")
+            .scan("DEPT")
+            .join(JoinRelType.LEFT,
+                builder.call(SqlStdOperatorTable.EQUALS,
+                    builder.field(2, "EMP", "DEPTNO"),
+                    builder.field(2, "DEPT", "DEPTNO")),
+                builder.call(SqlStdOperatorTable.EQUALS,
+                    builder.field(2, "EMP", "EMPNO"),
+                    builder.literal(123)))
+            .build();
+    final String expected = ""
+        + "LogicalJoin(condition=[AND(=($7, $8), =($0, 123))], joinType=[left])\n"
+        + "  LogicalTableScan(table=[[scott, EMP]])\n"
+        + "  LogicalTableScan(table=[[scott, DEPT]])\n";
+    assertThat(str(root), is(expected));
+  }
+
+  /** As {@link #testAliasPastTop()}. */
+  @Test public void testAliasPastTop2() {
+    // Equivalent SQL:
+    //   SELECT t1.EMPNO, t2.EMPNO, t3.DEPTNO
+    //   FROM emp t1
+    //   INNER JOIN emp t2 ON t1.EMPNO = t2.EMPNO
+    //   INNER JOIN dept t3 ON t1.DEPTNO = t3.DEPTNO
+    //     AND t2.JOB != t3.LOC
+    final RelBuilder builder = RelBuilder.create(config().build());
+    RelNode root =
+        builder.scan("EMP").as("t1")
+            .scan("EMP").as("t2")
+            .join(JoinRelType.INNER,
+                builder.equals(builder.field(2, "t1", "EMPNO"),
+                    builder.field(2, "t2", "EMPNO")))
+            .scan("DEPT").as("t3")
+            .join(JoinRelType.INNER,
+                builder.equals(builder.field(2, "t1", "DEPTNO"),
+                    builder.field(2, "t3", "DEPTNO")),
+                builder.not(
+                    builder.equals(builder.field(2, "t2", "JOB"),
+                        builder.field(2, "t3", "LOC"))))
+            .build();
+    // Cols:
+    // 0-7   EMP as t1
+    // 8-15  EMP as t2
+    // 16-18 DEPT as t3
+    final String expected = ""
+        + "LogicalJoin(condition=[AND(=($7, $16), <>($10, $18))], joinType=[inner])\n"
+        + "  LogicalJoin(condition=[=($0, $8)], joinType=[inner])\n"
+        + "    LogicalTableScan(table=[[scott, EMP]])\n"
+        + "    LogicalTableScan(table=[[scott, EMP]])\n"
+        + "  LogicalTableScan(table=[[scott, DEPT]])\n";
+    assertThat(str(root), is(expected));
+  }
+
   @Test public void testEmpty() {
     // Equivalent SQL:
     //   SELECT deptno, true FROM dept LIMIT 0

http://git-wip-us.apache.org/repos/asf/calcite/blob/ffe1a68d/site/_docs/algebra.md
----------------------------------------------------------------------
diff --git a/site/_docs/algebra.md b/site/_docs/algebra.md
index e651dd0..49d1631 100644
--- a/site/_docs/algebra.md
+++ b/site/_docs/algebra.md
@@ -235,7 +235,7 @@ field #8.
 
 But through the builder API, you specify which field of which input.
 To reference "SAL", internal field #5,
-write `builder.field(2, 0, "SAL")`
+write `builder.field(2, 0, "SAL")`, `builder.field(2, "EMP", "SAL")`,
 or `builder.field(2, 0, 5)`.
 This means "the field #5 of input #0 of two inputs".
 (Why does it need to know that there are two inputs? Because they are stored on
@@ -244,7 +244,7 @@ If we did not tell the builder that were two inputs, it would not know how deep
 to go for input #0.)
 
 Similarly, to reference "DNAME", internal field #9 (8 + 1),
-write `builder.field(2, 1, "DNAME")`
+write `builder.field(2, 1, "DNAME")`, `builder.field(2, "DEPT", "DNAME")`,
 or `builder.field(2, 1, 1)`.
 
 ### API summary
@@ -274,6 +274,7 @@ return the `RelBuilder`.
 | `union(all [, n])` | Creates a [Union]({{ site.apiRoot }}/org/apache/calcite/rel/core/Union.html) of the `n` (default two) most recent relational expressions.
 | `intersect(all [, n])` | Creates an [Intersect]({{ site.apiRoot }}/org/apache/calcite/rel/core/Intersect.html) of the `n` (default two) most recent relational expressions.
 | `minus(all)` | Creates a [Minus]({{ site.apiRoot }}/org/apache/calcite/rel/core/Minus.html) of the two most recent relational expressions.
+| `as(alias)` | Assigns the given alias to the top-most relational expression.
 
 Argument types:
 
@@ -327,6 +328,8 @@ added to the stack.
 | `field(fieldOrdinal)` | Reference, by ordinal, to a field of the top-most relational expression
 | `field(inputCount, inputOrdinal, fieldName)` | Reference, by name, to a field of the (`inputCount` - `inputOrdinal`)th relational expression
 | `field(inputCount, inputOrdinal, fieldOrdinal)` | Reference, by ordinal, to a field of the (`inputCount` - `inputOrdinal`)th relational expression
+| `field(inputCount, alias, fieldName)` | Reference, by table alias and field name, to a field at most `inputCount - 1` elements from the top of the stack
+| `field(alias, fieldName)` | Reference, by table alias and field name, to a field of the top-most relational expressions
 | `fields(fieldOrdinalList)` | List of expressions referencing input fields by ordinal
 | `fields(mapping)` | List of expressions referencing input fields by a given mapping
 | `fields(collation)` | List of expressions, `exprList`, such that `sort(exprList)` would replicate collation