diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdFunctionalDependency.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdFunctionalDependency.java index 15679c63c70..18732f908bf 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdFunctionalDependency.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdFunctionalDependency.java @@ -24,16 +24,19 @@ import org.apache.calcite.rel.core.Correlate; import org.apache.calcite.rel.core.Filter; import org.apache.calcite.rel.core.Join; +import org.apache.calcite.rel.core.JoinInfo; import org.apache.calcite.rel.core.JoinRelType; import org.apache.calcite.rel.core.Project; import org.apache.calcite.rel.core.SetOp; import org.apache.calcite.rel.core.TableScan; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rex.RexCall; import org.apache.calcite.rex.RexInputRef; import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.rex.RexNode; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.util.Arrow; import org.apache.calcite.util.ArrowSet; import org.apache.calcite.util.ImmutableBitSet; @@ -272,7 +275,8 @@ private ArrowSet getProjectionFD( ImmutableBitSet bitSet = expr instanceof RexInputRef ? ImmutableBitSet.of(((RexInputRef) expr).getIndex()) : inputBits[i]; - if (inputFdSet.implies(refIndex, bitSet)) { + if (typeSupportsGroupKeyInference(k.getType()) + && inputFdSet.implies(refIndex, bitSet)) { fdBuilder.addArrow(v, i); } }); @@ -292,7 +296,7 @@ private static void mapInputFDs(ArrowSet inputFdSet, // Map all determinant columns ImmutableBitSet mappedDeterminants = mapAllCols(determinants, mapping); - if (mappedDeterminants.isEmpty()) { + if (mappedDeterminants.isEmpty() && !determinants.isEmpty()) { continue; } @@ -345,18 +349,13 @@ private ArrowSet getAggregateFD(Aggregate rel, RelMetadataQuery mq) { ArrowSet inputFdSet = mq.getFDs(rel.getInput()); ImmutableBitSet groupSet = rel.getGroupSet(); + Mappings.TargetMapping inputToOutputMap = + Mappings.target(groupSet::indexOf, + rel.getInput().getRowType().getFieldCount(), rel.getGroupCount()); // Preserve input FDs that only involve group columns if (Aggregate.isSimple(rel)) { - for (Arrow inputFd : inputFdSet.getArrows()) { - ImmutableBitSet determinants = inputFd.getDeterminants(); - ImmutableBitSet dependents = inputFd.getDependents(); - - // Only preserve if both determinants and dependents are within group columns - if (groupSet.contains(determinants) && groupSet.contains(dependents)) { - fdBuilder.addArrow(determinants, dependents); - } - } + mapInputFDs(inputFdSet, inputToOutputMap, fdBuilder); // Add transitive dependencies within group columns for (int groupCol : groupSet) { @@ -364,7 +363,8 @@ private ArrowSet getAggregateFD(Aggregate rel, RelMetadataQuery mq) { ImmutableBitSet closure = inputFdSet.dependents(singleton); ImmutableBitSet groupDependents = closure.intersect(groupSet).except(singleton); if (!groupDependents.isEmpty()) { - fdBuilder.addArrow(singleton, groupDependents); + fdBuilder.addArrow(mapAllCols(singleton, inputToOutputMap), + mapAllCols(groupDependents, inputToOutputMap)); } } } @@ -373,7 +373,7 @@ private ArrowSet getAggregateFD(Aggregate rel, RelMetadataQuery mq) { if (!groupSet.isEmpty() && !rel.getAggCallList().isEmpty()) { ImmutableBitSet aggCols = ImmutableBitSet.range(rel.getGroupCount(), rel.getRowType().getFieldCount()); - fdBuilder.addArrow(groupSet, aggCols); + fdBuilder.addArrow(ImmutableBitSet.range(rel.getGroupCount()), aggCols); } return fdBuilder.build(); @@ -387,7 +387,7 @@ private ArrowSet getFilterFD(Filter rel, RelMetadataQuery mq) { ArrowSet.Builder fdBuilder = new ArrowSet.Builder(); // Adds equality dependencies from filter conditions. - addFDsFromEqualityCondition(rel.getCondition(), fdBuilder); + addBidirectionalFDsFromEqualityCondition(rel.getCondition(), fdBuilder); return fdBuilder.build().union(inputSet); } @@ -407,9 +407,12 @@ private ArrowSet getJoinFD(Join rel, RelMetadataQuery mq) { case INNER: case LEFT: case RIGHT: - ArrowSet.Builder joinFdBuilder = new ArrowSet.Builder() - .addArrowSet(leftFdSet.union(shiftFdSet(rightFdSet, leftFieldCount))); - addFDsFromEqualityCondition(rel.getCondition(), joinFdBuilder); + ArrowSet.Builder joinFdBuilder = new ArrowSet.Builder(); + addJoinInputFDs(leftFdSet, rel.getLeft(), 0, + joinType.generatesNullsOnLeft(), joinFdBuilder); + addJoinInputFDs(rightFdSet, rel.getRight(), leftFieldCount, + joinType.generatesNullsOnRight(), joinFdBuilder); + addFDsFromJoinCondition(rel, leftFieldCount, joinFdBuilder); return joinFdBuilder.build(); case SEMI: case ANTI: @@ -419,6 +422,41 @@ private ArrowSet getJoinFD(Join rel, RelMetadataQuery mq) { } } + /** + * Copies input dependencies into a join, optionally filtering dependencies + * that can be invalidated when the input is null-generated. + */ + private static void addJoinInputFDs(ArrowSet inputFdSet, RelNode input, + int offset, boolean nullGenerated, ArrowSet.Builder fdBuilder) { + for (Arrow inputFd : inputFdSet.getArrows()) { + if (nullGenerated && !hasNonNullableDeterminant(inputFd, input)) { + continue; + } + fdBuilder.addArrow(inputFd.getDeterminants().shift(offset), + inputFd.getDependents().shift(offset)); + } + } + + /** + * Returns whether a dependency's determinant contains a non-nullable input + * field. Such a determinant cannot collide with the all-NULL determinant of + * a padded outer-join row. + */ + private static boolean hasNonNullableDeterminant(Arrow fd, RelNode input) { + final int fieldCount = input.getRowType().getFieldCount(); + boolean hasNonNullableField = false; + for (int determinant : fd.getDeterminants()) { + if (determinant >= fieldCount) { + return false; + } + if (!input.getRowType().getFieldList().get(determinant) + .getType().isNullable()) { + hasNonNullableField = true; + } + } + return hasNonNullableField; + } + /** * Returns functional dependencies for Calc. */ @@ -428,50 +466,93 @@ private ArrowSet getCalcFD(Calc rel, RelMetadataQuery mq) { } /** - * Shifts column indices in functional dependencies (for right table in Joins). - * - * @param fdSet Functional dependency set - * @param offset Index offset - * @return Shifted functional dependency set + * Adds functional dependencies implied by a join condition. */ - private ArrowSet shiftFdSet(ArrowSet fdSet, int offset) { - ArrowSet.Builder shiftedFdSetBuilder = new ArrowSet.Builder(); - for (Arrow fd : fdSet.getArrows()) { - ImmutableBitSet shiftedDeterminants = fd.getDeterminants().shift(offset); - ImmutableBitSet shiftedDependents = fd.getDependents().shift(offset); - shiftedFdSetBuilder.addArrow(shiftedDeterminants, shiftedDependents); + private static void addFDsFromJoinCondition(Join rel, int leftFieldCount, + ArrowSet.Builder builder) { + final JoinRelType joinType = rel.getJoinType(); + if (joinType == JoinRelType.INNER) { + addBidirectionalFDsFromEqualityCondition(rel.getCondition(), builder); + return; } - return shiftedFdSetBuilder.build(); + + if (joinType == JoinRelType.LEFT || joinType == JoinRelType.RIGHT) { + final JoinInfo joinInfo = rel.analyzeCondition(); + if (!joinInfo.isEqui() || joinInfo.leftKeys.isEmpty() + || !fieldsSupportEqualityInference(rel.getLeft(), joinInfo.leftSet()) + || !fieldsSupportEqualityInference(rel.getRight(), joinInfo.rightSet())) { + return; + } + + final ImmutableBitSet leftKeys = joinInfo.leftSet(); + final ImmutableBitSet rightKeys = joinInfo.rightSet().shift(leftFieldCount); + if (joinType == JoinRelType.LEFT) { + builder.addArrow(leftKeys, rightKeys); + } else { + builder.addArrow(rightKeys, leftKeys); + } + return; + } + + throw new AssertionError("unsupported join type: " + joinType); } /** - * Extracts functional dependencies from equality and AND conditions. - * Handles col1 = col2, col1 IS NOT DISTINCT FROM col2, and AND conditions. + * Adds bidirectional dependencies for input-reference equalities in a + * condition. Callers are responsible for ensuring that every output row + * satisfies the condition, as is true for Filters and inner joins. */ - private static void addFDsFromEqualityCondition(RexNode condition, ArrowSet.Builder builder) { - if (!(condition instanceof RexCall)) { - return; - } + private static void addBidirectionalFDsFromEqualityCondition( + RexNode condition, ArrowSet.Builder builder) { + for (RexNode conjunct : RelOptUtil.conjunctions(condition)) { + if (!(conjunct instanceof RexCall)) { + continue; + } - RexCall call = (RexCall) condition; - if (call.getOperator().getKind() == SqlKind.EQUALS - || call.getOperator().getKind() == SqlKind.IS_NOT_DISTINCT_FROM) { + RexCall call = (RexCall) conjunct; + if (call.getOperator().getKind() != SqlKind.EQUALS + && call.getOperator().getKind() != SqlKind.IS_NOT_DISTINCT_FROM) { + continue; + } List operands = call.getOperands(); if (operands.size() == 2) { RexNode left = operands.get(0); RexNode right = operands.get(1); - if (left instanceof RexInputRef && right instanceof RexInputRef) { + if (left instanceof RexInputRef && right instanceof RexInputRef + && typeSupportsGroupKeyInference(left.getType()) + && typeSupportsGroupKeyInference(right.getType())) { int leftRef = ((RexInputRef) left).getIndex(); int rightRef = ((RexInputRef) right).getIndex(); builder.addBidirectionalArrow(leftRef, rightRef); } } - } else if (call.getOperator().getKind() == SqlKind.AND) { - for (RexNode operand : call.getOperands()) { - addFDsFromEqualityCondition(operand, builder); + } + } + + /** + * Returns whether equality on the given fields can safely imply a functional + * dependency for grouping purposes. + */ + private static boolean fieldsSupportEqualityInference(RelNode input, + ImmutableBitSet fields) { + for (int field : fields) { + if (!typeSupportsGroupKeyInference( + input.getRowType().getFieldList().get(field).getType())) { + return false; } } + return true; + } + + /** + * Returns whether a type can safely be used to infer that one grouping key + * determines another. Approximate numerics and intervals are unsafe, + * including when nested in rows, collections, or maps. + */ + private static boolean typeSupportsGroupKeyInference(RelDataType type) { + return !SqlTypeUtil.containsType(type, + t -> SqlTypeUtil.isApproximateNumeric(t) || SqlTypeUtil.isInterval(t)); } } diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java index c62fd580456..472063072fa 100644 --- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java +++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java @@ -62,6 +62,7 @@ import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import static com.google.common.base.Preconditions.checkArgument; @@ -405,6 +406,36 @@ public static boolean containsNullable(RelDataType type) { return false; } + /** + * Returns whether a type or any type nested within its fields, collection + * component, map key, or map value matches a predicate. + */ + public static boolean containsType(RelDataType type, + Predicate predicate) { + requireNonNull(type, "type"); + requireNonNull(predicate, "predicate"); + if (predicate.test(type)) { + return true; + } + if (type.isStruct()) { + for (RelDataTypeField field : type.getFieldList()) { + if (containsType(field.getType(), predicate)) { + return true; + } + } + } + final RelDataType componentType = type.getComponentType(); + if (componentType != null && containsType(componentType, predicate)) { + return true; + } + final RelDataType keyType = type.getKeyType(); + if (keyType != null && containsType(keyType, predicate)) { + return true; + } + final RelDataType valueType = type.getValueType(); + return valueType != null && containsType(valueType, predicate); + } + /** * Creates a RelDataType having the same type of the sourceRelDataType, * and the same nullability as the targetRelDataType. diff --git a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java index 1f08bd232e1..a01c221ea63 100644 --- a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java +++ b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java @@ -122,6 +122,38 @@ private RelDataType struct(RelDataType...relDataTypes) { return builder.build(); } + @Test void testContainsType() { + assertThat( + SqlTypeUtil.containsType(f.sqlInt, + SqlTypeUtil::isApproximateNumeric), is(false)); + assertThat( + SqlTypeUtil.containsType(f.sqlFloat, + SqlTypeUtil::isApproximateNumeric), is(true)); + assertThat( + SqlTypeUtil.containsType(struct(f.sqlInt, f.arrayFloat), + SqlTypeUtil::isApproximateNumeric), is(true)); + assertThat( + SqlTypeUtil.containsType(f.arrayOfArrayFloat, + SqlTypeUtil::isApproximateNumeric), is(true)); + assertThat( + SqlTypeUtil.containsType(f.multisetFloat, + SqlTypeUtil::isApproximateNumeric), is(true)); + + final RelDataType floatKeyMap = + f.typeFactory.createMapType(f.sqlFloat, f.sqlInt); + final RelDataType floatValueMap = + f.typeFactory.createMapType(f.sqlInt, f.sqlFloat); + assertThat( + SqlTypeUtil.containsType(floatKeyMap, + SqlTypeUtil::isApproximateNumeric), is(true)); + assertThat( + SqlTypeUtil.containsType(floatValueMap, + SqlTypeUtil::isApproximateNumeric), is(true)); + assertThat( + SqlTypeUtil.containsType(struct(f.arrayBigInt, f.mapOfInt), + SqlTypeUtil::isApproximateNumeric), is(false)); + } + @Test void testModifyTypeCoercionMappings() { SqlTypeMappingRules.Builder builder = SqlTypeMappingRules.builder(); final SqlTypeCoercionRule defaultRules = SqlTypeCoercionRule.instance(); diff --git a/core/src/test/java/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.java b/core/src/test/java/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.java index 3f65724a681..58feef244e1 100644 --- a/core/src/test/java/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.java +++ b/core/src/test/java/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.java @@ -72,6 +72,77 @@ private static RelOptFixture sql(String sql) { .check(); } + @Test void testKeepsGroupKeyFromPreservedSideOfLeftJoin() { + // The null-generating right key cannot determine the preserved left key: + // unmatched rows with different e.deptno values all have d.deptno = NULL. + final String sql = "select d.deptno, e.deptno, count(*) as c\n" + + "from emp e\n" + + "left join dept d on e.deptno = d.deptno\n" + + "group by d.deptno, e.deptno"; + + sql(sql).withRule(CoreRules.AGGREGATE_REMOVE_DUPLICATE_KEYS) + .checkUnchanged(); + } + + @Test void testKeepsFdFromNullGeneratingInputOfLeftJoin() { + // This FD comes from the right input rather than from the join condition. + // The right Project has a -> COALESCE(a, 1), but null padding adds a row + // with a = NULL and y = NULL alongside the matched a = NULL, y = 1 row. + final String sql = "select r.a, r.y, count(*) as c\n" + + "from (values (1), (2)) as l(z)\n" + + "left join (\n" + + " select z, a, coalesce(a, 1) as y\n" + + " from (values (1, cast(null as integer))) as v(z, a)\n" + + ") as r on l.z = r.z\n" + + "group by r.a, r.y"; + + sql(sql).withRule(CoreRules.AGGREGATE_REMOVE_DUPLICATE_KEYS) + .checkUnchanged(); + } + + @Test void testMapsFdForNonContiguousAggregateGroupSet() { + // AggregateProjectMergeRule changes the input group set to {1, 2, 3}, + // whose keys occupy output positions {0, 1, 2}. Map the input FD 1 -> 2 + // to output FD 0 -> 1, so the rule removes b rather than c. + final String sql = "select a, b, c, count(*) as n\n" + + "from (values (0, 1, 1, 10), (0, 1, 1, 20))\n" + + " as t(z, a, b, c)\n" + + "where a = b\n" + + "group by a, b, c"; + + sql(sql).withPreRule(CoreRules.AGGREGATE_PROJECT_MERGE) + .withRule(CoreRules.AGGREGATE_REMOVE_DUPLICATE_KEYS) + .check(); + } + + @Test void testKeepsDoubleGroupKeyInferredFromEquality() { + // SQL equality considers 0.0 and -0.0 equal, but Enumerable grouping + // distinguishes their Double keys. Therefore x = y does not prove that + // x determines y for the purpose of removing y from the GROUP BY. + final String sql = "select x, y, count(*) as c\n" + + "from (values\n" + + " (cast(0 as double), cast(0 as double)),\n" + + " (cast(0 as double), -cast(0 as double))) as t(x, y)\n" + + "where x = y\n" + + "group by x, y"; + + sql(sql).withRule(CoreRules.AGGREGATE_REMOVE_DUPLICATE_KEYS) + .checkUnchanged(); + } + + @Test void testKeepsDerivedGroupKeyForNestedDouble() { + // Determinism alone is insufficient for approximate values, including + // nested occurrences. Keep the derived key unless grouping equality is + // known to be congruent with the expression for ARRAY. + final String sql = "select x, x[1] as y, count(*) as c\n" + + "from (values (array[cast(0 as double)]),\n" + + " (array[cast(1 as double)])) as t(x)\n" + + "group by x, x[1]"; + + sql(sql).withRule(CoreRules.AGGREGATE_REMOVE_DUPLICATE_KEYS) + .checkUnchanged(); + } + @Test void testKeepsNonRedundantGroupKeys() { final String sql = "select deptno, job, count(*) as c\n" + "from emp\n" diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java index abb934457a0..9433cad0c9f 100644 --- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java @@ -710,6 +710,62 @@ final RelMetadataFixture sql(String sql) { assertThat(mq.determines(relNode, empNo, dname), is(Boolean.FALSE)); } + @Test void testFunctionalDependencyOuterJoinEqualityDirection() { + final RelNode leftJoin = sql("SELECT d.deptno AS null_generated_key," + + " e.deptno AS preserved_key\n" + + "FROM emp e\n" + + "LEFT JOIN dept d ON e.deptno = d.deptno").toRel(); + final RelMetadataQuery leftMq = leftJoin.getCluster().getMetadataQuery(); + + // For a pure LEFT equijoin, the preserved key determines the + // null-generated key, but the reverse does not hold for unmatched rows. + assertThat(leftMq.determines(leftJoin, 0, 1), is(Boolean.FALSE)); + assertThat(leftMq.determines(leftJoin, 1, 0), is(Boolean.TRUE)); + + final RelNode rightJoin = sql("SELECT d.deptno AS null_generated_key," + + " e.deptno AS preserved_key\n" + + "FROM dept d\n" + + "RIGHT JOIN emp e ON e.deptno = d.deptno").toRel(); + final RelMetadataQuery rightMq = rightJoin.getCluster().getMetadataQuery(); + + // RIGHT JOIN applies the same rule symmetrically. + assertThat(rightMq.determines(rightJoin, 0, 1), is(Boolean.FALSE)); + assertThat(rightMq.determines(rightJoin, 1, 0), is(Boolean.TRUE)); + + final RelNode residualJoin = sql("SELECT d.deptno AS null_generated_key," + + " e.deptno AS preserved_key\n" + + "FROM emp e\n" + + "LEFT JOIN dept d ON e.deptno = d.deptno" + + " AND e.empno > d.deptno").toRel(); + final RelMetadataQuery residualMq = residualJoin.getCluster().getMetadataQuery(); + + // With a residual predicate, the equality key alone does not determine + // whether a row is matched or padded. + assertThat(residualMq.determines(residualJoin, 1, 0), is(Boolean.FALSE)); + } + + @Test void testFunctionalDependencyFromNullGeneratingInput() { + final String nullableProject = "(SELECT z, a, COALESCE(a, 1) AS y\n" + + " FROM (VALUES (1, CAST(NULL AS INTEGER))) AS v(z, a))"; + + final RelNode leftJoin = sql("SELECT r.a, r.y\n" + + "FROM (VALUES (1), (2)) AS l(z)\n" + + "LEFT JOIN " + nullableProject + " AS r ON l.z = r.z").toRel(); + final RelMetadataQuery leftMq = leftJoin.getCluster().getMetadataQuery(); + + // The right Project has a -> COALESCE(a, 1), but LEFT JOIN padding adds + // a second a = NULL row whose y is NULL. + assertThat(leftMq.determines(leftJoin, 0, 1), is(Boolean.FALSE)); + + final RelNode rightJoin = sql("SELECT l.a, l.y\n" + + "FROM " + nullableProject + " AS l\n" + + "RIGHT JOIN (VALUES (1), (2)) AS r(z) ON l.z = r.z").toRel(); + final RelMetadataQuery rightMq = rightJoin.getCluster().getMetadataQuery(); + + // RIGHT JOIN invalidates the corresponding dependency symmetrically. + assertThat(rightMq.determines(rightJoin, 0, 1), is(Boolean.FALSE)); + } + @Test void testFunctionalDependencySort() { final String sql = "select empno, ename, deptno, sal" + " from emp order by empno, deptno"; diff --git a/core/src/test/resources/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.xml b/core/src/test/resources/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.xml index f01bb8e675d..5f164e4e566 100644 --- a/core/src/test/resources/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/AggregateRemoveDuplicateKeysRuleTest.xml @@ -16,6 +16,85 @@ ~ limitations under the License. --> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + LogicalAggregate(group=[{0, 1}], C=[COUNT()]) LogicalProject(DEPTNO=[$7], JOB=[$2]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + + + + + + + + + + +