julianhyde commented on code in PR #3495:
URL: https://github.com/apache/calcite/pull/3495#discussion_r1406958858
##########
core/src/main/java/org/apache/calcite/rel/metadata/BuiltInMetadata.java:
##########
@@ -83,13 +83,18 @@ public interface UniqueKeys extends Metadata {
* represented as an {@link org.apache.calcite.util.ImmutableBitSet}, where
* each bit position represents a 0-based output column ordinal.
*
+ * <p>Note that a key plus other columns is still unique. Therefore a table
+ * with all columns unique has a unique key consisting of the empty set.
+ * This is the case with all single-row tables.
Review Comment:
Not true. The tuple with zero columns has only one possible value. Therefore
it is unique only in tables with 0 or 1 rows. (In such tables every combination
of columns is unique.)
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java:
##########
@@ -533,6 +535,36 @@ public RelOptPredicateList getPredicates(Exchange exchange,
return mq.getPulledUpPredicates(input);
}
+ /**
+ * Infers predicates for a Values.
+ */
Review Comment:
A comment
```
The predicates on T (x, y, z) with rows (1, 2, null), (1, 2, null), (5, 2,
null) are 'y = 2' and 'z is null'
```
would be useful.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -478,17 +525,78 @@ private static ImmutableBitSet
decorateWithConstantColumnsFromPredicates(
ImmutableBitSet checkingColumns, RelNode rel, RelMetadataQuery mq) {
final RelOptPredicateList predicates = mq.getPulledUpPredicates(rel);
if (!RelOptPredicateList.isEmpty(predicates)) {
- final Set<Integer> constantIndexes = new HashSet<>();
- predicates.constantMap.keySet().forEach(rex -> {
- if (rex instanceof RexInputRef) {
- constantIndexes.add(((RexInputRef) rex).getIndex());
- }
- });
+ ImmutableBitSet constantIndexes = getConstantColumnSet(predicates);
if (!constantIndexes.isEmpty()) {
return checkingColumns.union(ImmutableBitSet.of(constantIndexes));
}
}
// If no constant columns deduced, return the original "checkingColumns".
return checkingColumns;
}
+
+ /**
+ * Return the set of columns that are set to a constant literal or a scalar
query (as
Review Comment:
s/Return/Returns/
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -68,6 +79,15 @@ public class RelMdColumnUniqueness
ReflectiveRelMetadataProvider.reflectiveSource(
new RelMdColumnUniqueness(),
BuiltInMetadata.ColumnUniqueness.Handler.class);
+ /**
+ * If a row has a unique column, then an aggregation that returns a value
from that column is
+ * also unique. These are the aggregations that do that. Note that this
quality is not
+ * guaranteed in the presence of an OVER clause. NOTE: if a multi-argument
function is added,
+ * methods that use this Set must be enhanced to select the appropriate
column to pass through.
+ */
+ static final Set<SqlKind> PASSTHROUGH_AGGREGATIONS =
Review Comment:
The first sentence should be a description of the field. "The set of
aggregate functions A such that if x is unique then A(x) will also be unique.
An aggregate function with this property is called 'passthrough'."
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -478,17 +525,78 @@ private static ImmutableBitSet
decorateWithConstantColumnsFromPredicates(
ImmutableBitSet checkingColumns, RelNode rel, RelMetadataQuery mq) {
final RelOptPredicateList predicates = mq.getPulledUpPredicates(rel);
if (!RelOptPredicateList.isEmpty(predicates)) {
- final Set<Integer> constantIndexes = new HashSet<>();
- predicates.constantMap.keySet().forEach(rex -> {
- if (rex instanceof RexInputRef) {
- constantIndexes.add(((RexInputRef) rex).getIndex());
- }
- });
+ ImmutableBitSet constantIndexes = getConstantColumnSet(predicates);
if (!constantIndexes.isEmpty()) {
return checkingColumns.union(ImmutableBitSet.of(constantIndexes));
}
}
// If no constant columns deduced, return the original "checkingColumns".
return checkingColumns;
}
+
+ /**
+ * Return the set of columns that are set to a constant literal or a scalar
query (as
+ * in a correlated subquery). Examples of constants are {@code x} in the
following:
+ * <pre>SELECT x FROM table WHERE x = 5</pre>
+ * and
+ * <pre>SELECT x, y FROM table WHERE x = (SELECT MAX(x) FROM table)</pre>
+ * <p/>
Review Comment:
Remove `</p>`. We don't use end-of-paragraph markers.
Add `<p>` at start of line, after blank line.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -526,12 +525,78 @@ private static ImmutableBitSet
decorateWithConstantColumnsFromPredicates(
ImmutableBitSet checkingColumns, RelNode rel, RelMetadataQuery mq) {
final RelOptPredicateList predicates = mq.getPulledUpPredicates(rel);
if (!RelOptPredicateList.isEmpty(predicates)) {
- ImmutableBitSet invariantIndexes = predicates.getInvariantColumnSet();
- if (!invariantIndexes.isEmpty()) {
- return checkingColumns.union(ImmutableBitSet.of(invariantIndexes));
+ ImmutableBitSet constantIndexes = getConstantColumnSet(predicates);
+ if (!constantIndexes.isEmpty()) {
+ return checkingColumns.union(ImmutableBitSet.of(constantIndexes));
}
}
- // If no invariant columns deduced, return the original "checkingColumns".
+ // If no constant columns deduced, return the original "checkingColumns".
return checkingColumns;
}
+
+ /**
+ * Return the set of columns that are set to a constant literal or a scalar
query (as
+ * in a correlated subquery). Examples of constants are {@code x} in the
following:
+ * <pre>SELECT x FROM table WHERE x = 5</pre>
+ * and
+ * <pre>SELECT x, y FROM table WHERE x = (SELECT MAX(x) FROM table)</pre>
+ * <p/>
+ * NOTE: Subqueries that reference correlating variables are not considered
constant:
+ * <pre>SELECT x, y FROM table A WHERE x = (SELECT MAX(x) FROM table B WHERE
A.y = B.y)</pre>
+ */
+ static ImmutableBitSet getConstantColumnSet(RelOptPredicateList
relOptPredicateList) {
+ ImmutableBitSet.Builder builder = ImmutableBitSet.builder();
+ relOptPredicateList.constantMap.keySet()
+ .stream()
+ .filter(RexInputRef.class::isInstance)
+ .map(RexInputRef.class::cast)
+ .map(RexSlot::getIndex)
+ .forEach(builder::set);
+
+ relOptPredicateList.pulledUpPredicates.forEach(rex -> {
+ if (rex.getKind() == SqlKind.EQUALS
+ || rex.getKind() == SqlKind.IS_NOT_DISTINCT_FROM) {
+ List<RexNode> ops = ((RexCall) rex).getOperands();
+ RexNode op0 = ops.get(0);
+ RexNode op1 = ops.get(1);
+ addInputRefIfOtherConstant(builder, op0, op1);
+ addInputRefIfOtherConstant(builder, op1, op0);
+ }
+ });
+
+ return builder.build();
+ }
+
+ private static void addInputRefIfOtherConstant(ImmutableBitSet.Builder
builder, RexNode inputRef,
+ RexNode other) {
+ if (inputRef instanceof RexInputRef
+ && (other.getKind() == SqlKind.LITERAL ||
isConstantScalarQuery(other))) {
+ builder.set(((RexInputRef) inputRef).getIndex());
+ }
+ }
+
+ /**
+ * Returns whether the supplied {@link RexNode} is a constant scalar
subquery - one that does not
+ * reference any correlating variables.
+ */
+ private static boolean isConstantScalarQuery(RexNode rexNode) {
+ if (rexNode.getKind() == SqlKind.SCALAR_QUERY) {
+ MutableBoolean hasCorrelatedVars = new MutableBoolean(false);
+ ((RexSubQuery) rexNode).rel.accept(new RelShuttleImpl() {
+ @Override public RelNode visit(final LogicalFilter filter) {
+ filter.getCondition().accept(new RexShuttle() {
+ @Override public RexNode visitFieldAccess(final RexFieldAccess
fieldAccess) {
+ if (fieldAccess.getReferenceExpr().getKind() ==
SqlKind.CORREL_VARIABLE) {
+ hasCorrelatedVars.setTrue();
+ }
+ return super.visitFieldAccess(fieldAccess);
+ }
+ });
+ return super.visit(filter);
+ }
Review Comment:
Could you use `RexUtil.containsCorrelation(filter.getCondition())`?
##########
core/src/main/java/org/apache/calcite/sql/SqlOverOperator.java:
##########
@@ -115,6 +115,7 @@ public SqlOverOperator() {
// Copied from validateOperands
validator.setValidatedNodeType(call, ret);
+ // TODO: Should the type of the agg change just because it is an OVER
operand?
Review Comment:
Yes, we don't want to add TODOs. They never get done.
If you have a question, ask it in jira.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
