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 2015/09/10 02:48:52 UTC
[2/2] incubator-calcite git commit: [CALCITE-881] Allow
schema.table.column references in GROUP BY
[CALCITE-881] Allow schema.table.column references in GROUP BY
To keep the horse in front of the cart, move a few fields and methods from
AggregatingSelectScope to a new class Resolved, which is created after the
scope tree is built, i.e. during validation.
Project: http://git-wip-us.apache.org/repos/asf/incubator-calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/16512edc
Tree: http://git-wip-us.apache.org/repos/asf/incubator-calcite/tree/16512edc
Diff: http://git-wip-us.apache.org/repos/asf/incubator-calcite/diff/16512edc
Branch: refs/heads/master
Commit: 16512edc1308b6643bbdba901996a4b0a5c014e9
Parents: 600e1ca
Author: Julian Hyde <jh...@apache.org>
Authored: Wed Sep 9 16:57:12 2015 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Wed Sep 9 17:00:16 2015 -0700
----------------------------------------------------------------------
.../sql/fun/SqlAbstractGroupFunction.java | 3 +-
.../sql/validate/AggregatingSelectScope.java | 106 ++++++++++++-------
.../calcite/sql/validate/DelegatingScope.java | 10 ++
.../calcite/sql2rel/SqlToRelConverter.java | 32 +++---
.../apache/calcite/test/SqlValidatorTest.java | 23 ++--
core/src/test/resources/sql/misc.oq | 31 ++++++
6 files changed, 142 insertions(+), 63 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/16512edc/core/src/main/java/org/apache/calcite/sql/fun/SqlAbstractGroupFunction.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlAbstractGroupFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlAbstractGroupFunction.java
index 0d9d2ea..443bd4f 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlAbstractGroupFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlAbstractGroupFunction.java
@@ -63,6 +63,7 @@ public class SqlAbstractGroupFunction extends SqlFunction {
super.validateCall(call, validator, scope, operandScope);
final SelectScope selectScope =
SqlValidatorUtil.getEnclosingSelectScope(scope);
+ assert selectScope != null;
final SqlSelect select = selectScope.getNode();
if (!validator.isAggregate(select)) {
throw validator.newValidationError(call,
@@ -81,7 +82,7 @@ public class SqlAbstractGroupFunction extends SqlFunction {
} else {
operand = validator.expand(operand, scope);
}
- if (!aggregatingSelectScope.isGroupingExpr(operand)) {
+ if (!aggregatingSelectScope.resolved.get().isGroupingExpr(operand)) {
throw validator.newValidationError(operand,
Static.RESOURCE.groupingArgument(getName()));
}
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/16512edc/core/src/main/java/org/apache/calcite/sql/validate/AggregatingSelectScope.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/AggregatingSelectScope.java b/core/src/main/java/org/apache/calcite/sql/validate/AggregatingSelectScope.java
index 58fda81..580e71c 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/AggregatingSelectScope.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/AggregatingSelectScope.java
@@ -25,12 +25,14 @@ import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlSelect;
import org.apache.calcite.util.ImmutableBitSet;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -52,15 +54,21 @@ public class AggregatingSelectScope
private final boolean distinct;
/** Use while under construction. */
- private final List<SqlNode> temporaryGroupExprList = Lists.newArrayList();
+ private List<SqlNode> temporaryGroupExprList;
- /** Use after construction is complete. Assigned from
- * {@link #temporaryGroupExprList} towards the end of the constructor. */
- public final ImmutableList<SqlNode> groupExprList;
- public final ImmutableBitSet groupSet;
- public final ImmutableList<ImmutableBitSet> groupSets;
- public final boolean indicator;
- public final Map<Integer, Integer> groupExprProjection;
+ public final Supplier<Resolved> resolved =
+ Suppliers.memoize(
+ new Supplier<Resolved>() {
+ public Resolved get() {
+ assert temporaryGroupExprList == null;
+ temporaryGroupExprList = new ArrayList<>();
+ try {
+ return resolve();
+ } finally {
+ temporaryGroupExprList = null;
+ }
+ }
+ });
//~ Constructors -----------------------------------------------------------
@@ -81,7 +89,12 @@ public class AggregatingSelectScope
super(selectScope);
this.select = select;
this.distinct = distinct;
- final Map<Integer, Integer> groupExprProjection = Maps.newHashMap();
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ private Resolved resolve() {
+ final Map<Integer, Integer> groupExprProjection = new HashMap<>();
final ImmutableList.Builder<ImmutableList<ImmutableBitSet>> builder =
ImmutableList.builder();
if (select.getGroup() != null) {
@@ -95,8 +108,6 @@ public class AggregatingSelectScope
groupExprProjection, builder, groupExpr);
}
}
- this.groupExprList = ImmutableList.copyOf(temporaryGroupExprList);
- this.groupExprProjection = ImmutableMap.copyOf(groupExprProjection);
final Set<ImmutableBitSet> flatGroupSets =
Sets.newTreeSet(ImmutableBitSet.COMPARATOR);
@@ -109,13 +120,10 @@ public class AggregatingSelectScope
flatGroupSets.add(ImmutableBitSet.of());
}
- this.groupSet = ImmutableBitSet.range(groupExprList.size());
- this.groupSets = ImmutableList.copyOf(flatGroupSets);
- this.indicator = !groupSets.equals(ImmutableList.of(groupSet));
+ return new Resolved(temporaryGroupExprList, flatGroupSets,
+ groupExprProjection);
}
- //~ Methods ----------------------------------------------------------------
-
/**
* Returns the expressions that are in the GROUP BY clause (or the SELECT
* DISTINCT clause, if distinct) and that can therefore be referenced
@@ -141,10 +149,11 @@ public class AggregatingSelectScope
}
return groupExprs.build();
} else if (select.getGroup() != null) {
- if (groupExprList != null) {
- return groupExprList;
- } else {
+ if (temporaryGroupExprList != null) {
+ // we are in the middle of resolving
return ImmutableList.copyOf(temporaryGroupExprList);
+ } else {
+ return resolved.get().groupExprList;
}
} else {
return ImmutableList.of();
@@ -155,11 +164,6 @@ public class AggregatingSelectScope
return select;
}
- /** Returns whether a field should be nullable due to grouping sets. */
- public boolean isNullable(int i) {
- return i < groupExprList.size() && !allContain(groupSets, i);
- }
-
private static boolean allContain(List<ImmutableBitSet> bitSets, int bit) {
for (ImmutableBitSet bitSet : bitSets) {
if (!bitSet.get(bit)) {
@@ -170,9 +174,10 @@ public class AggregatingSelectScope
}
@Override public RelDataType nullifyType(SqlNode node, RelDataType type) {
- for (Ord<SqlNode> groupExpr : Ord.zip(groupExprList)) {
+ final Resolved r = this.resolved.get();
+ for (Ord<SqlNode> groupExpr : Ord.zip(r.groupExprList)) {
if (groupExpr.e.equalsDeep(node, false)) {
- if (isNullable(groupExpr.i)) {
+ if (r.isNullable(groupExpr.i)) {
return validator.getTypeFactory().createTypeWithNullability(type,
true);
}
@@ -235,19 +240,44 @@ public class AggregatingSelectScope
checkAggregateExpr(expr, true);
}
- /** Returns whether a given expression is equal to one of the grouping
- * expressions. Determines whether it is valid as an operand to GROUPING. */
- public boolean isGroupingExpr(SqlNode operand) {
- return lookupGroupingExpr(operand) >= 0;
- }
+ /** Information about an aggregating scope that can only be determined
+ * after validation has occurred. Therefore it cannot be populated when
+ * the scope is created. */
+ public class Resolved {
+ public final ImmutableList<SqlNode> groupExprList;
+ public final ImmutableBitSet groupSet;
+ public final ImmutableList<ImmutableBitSet> groupSets;
+ public final boolean indicator;
+ public final Map<Integer, Integer> groupExprProjection;
- public int lookupGroupingExpr(SqlNode operand) {
- for (Ord<SqlNode> groupExpr : Ord.zip(groupExprList)) {
- if (operand.equalsDeep(groupExpr.e, false)) {
- return groupExpr.i;
+ Resolved(List<SqlNode> groupExprList, Iterable<ImmutableBitSet> groupSets,
+ Map<Integer, Integer> groupExprProjection) {
+ this.groupExprList = ImmutableList.copyOf(groupExprList);
+ this.groupSet = ImmutableBitSet.range(groupExprList.size());
+ this.groupSets = ImmutableList.copyOf(groupSets);
+ this.indicator = !this.groupSets.equals(ImmutableList.of(groupSet));
+ this.groupExprProjection = ImmutableMap.copyOf(groupExprProjection);
+ }
+
+ /** Returns whether a field should be nullable due to grouping sets. */
+ public boolean isNullable(int i) {
+ return i < groupExprList.size() && !allContain(groupSets, i);
+ }
+
+ /** Returns whether a given expression is equal to one of the grouping
+ * expressions. Determines whether it is valid as an operand to GROUPING. */
+ public boolean isGroupingExpr(SqlNode operand) {
+ return lookupGroupingExpr(operand) >= 0;
+ }
+
+ public int lookupGroupingExpr(SqlNode operand) {
+ for (Ord<SqlNode> groupExpr : Ord.zip(groupExprList)) {
+ if (operand.equalsDeep(groupExpr.e, false)) {
+ return groupExpr.i;
+ }
}
+ return -1;
}
- return -1;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/16512edc/core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java b/core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java
index dc50668..d5049e7 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java
@@ -198,6 +198,16 @@ public abstract class DelegatingScope implements SqlValidatorScope {
identifier = identifier.setName(j, field.getName());
fromRowType = field.getType();
}
+ if (i > 1) {
+ // Simplify overqualified identifiers.
+ // For example, schema.emp.deptno becomes emp.deptno.
+ //
+ // It is safe to convert schema.emp or database.schema.emp to emp
+ // because it would not have resolved if the FROM item had an alias. The
+ // following query is invalid:
+ // SELECT schema.emp.deptno FROM schema.emp AS e
+ identifier = identifier.getComponent(i - 1, identifier.names.size());
+ }
return SqlQualified.create(this, i, fromNs, identifier);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/16512edc/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
index 7f73a08..b51a0dc 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -2461,11 +2461,12 @@ public class SqlToRelConverter {
// build a map to remember the projections from the top scope to the
// output of the current root.
//
- // Currently farrago allows expressions, not just column references in
- // group by list. This is not SQL 2003 compliant.
+ // Calcite allows expressions, not just column references in
+ // group by list. This is not SQL 2003 compliant, but hey.
final AggregatingSelectScope scope = aggConverter.aggregatingSelectScope;
- for (SqlNode groupExpr : scope.groupExprList) {
+ final AggregatingSelectScope.Resolved r = scope.resolved.get();
+ for (SqlNode groupExpr : r.groupExprList) {
aggConverter.addGroupExpr(groupExpr);
}
@@ -2496,8 +2497,8 @@ public class SqlToRelConverter {
// at all. The rest of the system doesn't like 0-tuples, so we
// select a dummy constant here.
preExprs =
- Collections.singletonList(
- (RexNode) rexBuilder.makeExactLiteral(BigDecimal.ZERO));
+ ImmutableList.<RexNode>of(
+ rexBuilder.makeExactLiteral(BigDecimal.ZERO));
preNames = Collections.singletonList(null);
}
@@ -2511,7 +2512,7 @@ public class SqlToRelConverter {
preNames,
true),
false);
- bb.mapRootRelToFieldProjection.put(bb.root, scope.groupExprProjection);
+ bb.mapRootRelToFieldProjection.put(bb.root, r.groupExprProjection);
// REVIEW jvs 31-Oct-2007: doesn't the declaration of
// monotonicity here assume sort-based aggregation at
@@ -2526,8 +2527,8 @@ public class SqlToRelConverter {
// Add the aggregator
bb.setRoot(
- createAggregate(bb, aggConverter.aggregatingSelectScope.indicator,
- scope.groupSet, scope.groupSets, aggConverter.getAggCalls()),
+ createAggregate(bb, r.indicator, r.groupSet, r.groupSets,
+ aggConverter.getAggCalls()),
false);
// Generate NULL values for rolled-up not-null fields.
@@ -2540,7 +2541,7 @@ public class SqlToRelConverter {
for (RelDataTypeField field : aggregate.getRowType().getFieldList()) {
final int i = field.getIndex();
final RexNode rex;
- if (i < groupCount && scope.isNullable(i)) {
+ if (i < groupCount && r.isNullable(i)) {
++converted;
rex = rexBuilder.makeCall(SqlStdOperatorTable.CASE,
@@ -2562,7 +2563,7 @@ public class SqlToRelConverter {
}
}
- bb.mapRootRelToFieldProjection.put(bb.root, scope.groupExprProjection);
+ bb.mapRootRelToFieldProjection.put(bb.root, r.groupExprProjection);
// Replace subqueries in having here and modify having to use
// the replaced expressions
@@ -4498,11 +4499,13 @@ public class SqlToRelConverter {
filterArg,
type,
nameMap.get(outerCall.toString()));
+ final AggregatingSelectScope.Resolved r =
+ aggregatingSelectScope.resolved.get();
RexNode rex =
rexBuilder.addAggCall(
aggCall,
groupExprs.size(),
- aggregatingSelectScope.indicator,
+ r.indicator,
aggCalls,
aggCallMapping,
argTypes);
@@ -4547,7 +4550,7 @@ public class SqlToRelConverter {
case GROUPING_ID:
case GROUP_ID:
final RelDataType type = validator.getValidatedNodeType(call);
- if (!aggregatingSelectScope.indicator) {
+ if (!aggregatingSelectScope.resolved.get().indicator) {
return rexBuilder.makeExactLiteral(
TWO.pow(effectiveArgCount(call)).subtract(BigDecimal.ONE), type);
} else {
@@ -4590,9 +4593,10 @@ public class SqlToRelConverter {
private RexNode bitValue(RexNode previous, RelDataType type, int x,
int shift) {
+ final AggregatingSelectScope.Resolved r =
+ aggregatingSelectScope.resolved.get();
RexNode node = rexBuilder.makeCall(SqlStdOperatorTable.CASE,
- rexBuilder.makeInputRef(bb.root,
- aggregatingSelectScope.groupExprList.size() + x),
+ rexBuilder.makeInputRef(bb.root, r.groupExprList.size() + x),
rexBuilder.makeExactLiteral(BigDecimal.ONE, type),
rexBuilder.makeExactLiteral(BigDecimal.ZERO, type));
if (shift > 0) {
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/16512edc/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 78d9e1c..e161b26 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -4780,9 +4780,13 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
.fails("Table 'SALES.BAD' not found");
}
- @Ignore("does not work yet")
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-881">[CALCITE-881]
+ * Allow schema.table.column references in GROUP BY</a>. */
@Test public void testSchemaTableColumnInGroupBy() {
- sql("select 1 from sales.emp group by sales.emp.deptno").ok(); // TODO:
+ sql("select 1 from sales.emp group by sales.emp.deptno").ok();
+ sql("select deptno from sales.emp group by sales.emp.deptno").ok();
+ sql("select deptno + 1 from sales.emp group by sales.emp.deptno").ok();
}
@Test public void testInvalidGroupBy() {
@@ -6043,14 +6047,13 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
.type("RecordType(INTEGER NOT NULL DEPTNO, INTEGER EMPNO) NOT NULL");
}
- @Test public void testGroupByCorrelatedColumnFails() {
- // -- this is not sql 2003 standard
- // -- see sql2003 part2, 7.9
- checkFails(
- "select count(*)\n"
- + "from emp\n"
- + "where exists (select count(*) from dept group by ^emp^.empno)",
- "Table 'EMP' not found");
+ @Test public void testGroupByCorrelatedColumn() {
+ // This is not sql 2003 standard; see sql2003 part2, 7.9
+ // But the extension seems harmless.
+ final String sql = "select count(*)\n"
+ + "from emp\n"
+ + "where exists (select count(*) from dept group by emp.empno)";
+ sql(sql).ok();
}
@Test public void testGroupExpressionEquivalence() {
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/16512edc/core/src/test/resources/sql/misc.oq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/misc.oq b/core/src/test/resources/sql/misc.oq
index 1d1bd57..2b9b3c9 100644
--- a/core/src/test/resources/sql/misc.oq
+++ b/core/src/test/resources/sql/misc.oq
@@ -33,6 +33,37 @@ from "hr"."emps";
!ok
+# [CALCITE-881] Allow schema.table.column references in GROUP BY
+select "hr"."emps"."empid", count(*) as c
+from "hr"."emps"
+group by "hr"."emps"."empid";
++-------+---+
+| empid | C |
++-------+---+
+| 100 | 1 |
+| 110 | 1 |
+| 150 | 1 |
+| 200 | 1 |
++-------+---+
+(4 rows)
+
+!ok
+
+select distinct "hr"."emps"."empid" + 1 as e
+from "hr"."emps"
+group by "hr"."emps"."empid";
++-----+
+| E |
++-----+
+| 101 |
+| 111 |
+| 151 |
+| 201 |
++-----+
+(4 rows)
+
+!ok
+
# [CALCITE-307] CAST(timestamp AS DATE) gives ClassCastException
# Based on [DRILL-1051]
with data(c_row, c_timestamp) as (select * from (values