This is an automated email from the ASF dual-hosted git repository.
rubenada pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 6e84512ac4 [CALCITE-7795] SemanticTable FilterRequirement is dropped
by certain query shapes
6e84512ac4 is described below
commit 6e84512ac45a328beca437e2a11fabc3f271dc45
Author: Ruben Quesada Lopez <[email protected]>
AuthorDate: Fri Sep 18 12:04:11 2026 +0100
[CALCITE-7795] SemanticTable FilterRequirement is dropped by certain query
shapes
---
.../calcite/sql/validate/AbstractNamespace.java | 18 +
.../calcite/sql/validate/AliasNamespace.java | 3 +
.../calcite/sql/validate/DelegatingNamespace.java | 4 +
.../calcite/sql/validate/FilterRequirement.java | 87 +++-
.../sql/validate/MatchRecognizeNamespace.java | 8 +
.../calcite/sql/validate/PivotNamespace.java | 8 +
.../calcite/sql/validate/ProcedureNamespace.java | 21 +
.../calcite/sql/validate/SetopNamespace.java | 14 +
.../calcite/sql/validate/SqlValidatorImpl.java | 321 ++++++++------
.../calcite/sql/validate/UnnestNamespace.java | 3 +
.../calcite/sql/validate/UnpivotNamespace.java | 8 +
.../org/apache/calcite/test/SqlValidatorTest.java | 473 +++++++++++++++++++++
.../test/catalog/MustFilterMockCatalogReader.java | 9 +
13 files changed, 839 insertions(+), 138 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/AbstractNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/AbstractNamespace.java
index 4bfa1a80e0..707c460f80 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/AbstractNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/AbstractNamespace.java
@@ -170,6 +170,24 @@ abstract class AbstractNamespace implements
SqlValidatorNamespace {
"filterRequirement (maybe validation is not complete?)");
}
+ /** Requires that a source namespace has no pending "must-filter"
obligations.
+ *
+ * <p>Namespaces such as PIVOT, UNPIVOT, MATCH_RECOGNIZE and table function
+ * calls transform the rows of their source in a way that does not preserve
+ * column positions, and therefore cannot re-express the source's
+ * {@link FilterRequirement} in terms of their own output columns. Rather
+ * than silently dropping the requirement (which would let a query read a
+ * {@link SemanticTable}'s rows with the must-filter enforcement bypassed),
+ * such namespaces fail closed: the source's must-filter fields have to be
+ * filtered (or defused by a bypass field) within the source query itself.
+ *
+ * @param sourceNs Namespace whose rows this namespace transforms
+ * @param node Node to which a validation error should be attached
+ */
+ protected void checkFilterRequirementSatisfied(SqlValidatorNamespace
sourceNs, SqlNode node) {
+ validator.checkFilterRequirementSatisfied(sourceNs, node);
+ }
+
@Override public SqlMonotonicity getMonotonicity(String columnName) {
return SqlMonotonicity.NOT_MONOTONIC;
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
index 99f84154e7..3e1f29a939 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
@@ -84,6 +84,9 @@ protected AliasNamespace(
final SqlValidatorNamespace childNs =
validator.getNamespaceOrThrow(operands.get(0));
final RelDataType rowType0 = childNs.getRowTypeSansSystemColumns();
+ // An alias namespace renames columns positionally (or, without a column
+ // list, not at all), so the child's filter requirement applies unchanged
+ filterRequirement = childNs.getFilterRequirement();
final RelDataType rowType;
if (rowType0.isStruct()) {
rowType = rowType0;
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/DelegatingNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/DelegatingNamespace.java
index 21f59a3dfd..f8af0a1e78 100644
---
a/core/src/main/java/org/apache/calcite/sql/validate/DelegatingNamespace.java
+++
b/core/src/main/java/org/apache/calcite/sql/validate/DelegatingNamespace.java
@@ -96,6 +96,10 @@ protected DelegatingNamespace(SqlValidatorNamespace
namespace) {
return namespace.getMonotonicExprs();
}
+ @Override public FilterRequirement getFilterRequirement() {
+ return namespace.getFilterRequirement();
+ }
+
@Override public SqlMonotonicity getMonotonicity(String columnName) {
return namespace.getMonotonicity(columnName);
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/FilterRequirement.java
b/core/src/main/java/org/apache/calcite/sql/validate/FilterRequirement.java
index 30dbe2229a..e9032e8a46 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/FilterRequirement.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/FilterRequirement.java
@@ -18,6 +18,7 @@
import org.apache.calcite.util.ImmutableBitSet;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import java.util.Set;
@@ -96,13 +97,22 @@
* The query is valid because {@link #remnantFilterFields} is now empty.
* </ol>
*
+ * <p><b>Notes on groups</b>
+ *
+ * <p>A bypass field only defuses the requirements of the table that it
+ * belongs to. When a query reads several tables, for example a join of
+ * {@code t} and {@code u}, the filter requirement keeps the requirements of
+ * each table instance in a separate {@link Group}, so that a filter on a
+ * bypass field of {@code t} cannot defuse the requirements of {@code u}.
+ * {@link #filterFields}, {@link #bypassFields} and
+ * {@link #remnantFilterFields} are the union of those of the groups.
+ *
* @see SqlValidatorNamespace#getFilterRequirement()
*/
public class FilterRequirement {
/** Empty filter requirement. */
public static final FilterRequirement EMPTY =
- new FilterRequirement(ImmutableBitSet.of(), ImmutableBitSet.of(),
- ImmutableSet.of());
+ new FilterRequirement(ImmutableList.of());
/** Ordinals (in the row type) of the "must-filter" fields,
* fields that must be filtered in a query. */
@@ -118,8 +128,12 @@ public class FilterRequirement {
* on a bypass field in the enclosing query. */
public final ImmutableSet<SqlQualified> remnantFilterFields;
+ /** Requirements of each table instance that has not been defused. */
+ public final ImmutableList<Group> groups;
+
/**
- * Creates a {@code FilterRequirement}.
+ * Creates a {@code FilterRequirement} with the requirements of a single
+ * table instance.
*
* @param filterFields Ordinals of the "must-filter" fields
* @param bypassFields Ordinals of the "bypass" fields
@@ -128,8 +142,69 @@ public class FilterRequirement {
*/
FilterRequirement(Iterable<Integer> filterFields,
Iterable<Integer> bypassFields, Set<SqlQualified> remnantFilterFields) {
- this.filterFields = ImmutableBitSet.of(filterFields);
- this.bypassFields = ImmutableBitSet.of(bypassFields);
- this.remnantFilterFields = ImmutableSet.copyOf(remnantFilterFields);
+ this(
+ ImmutableList.of(
+ new Group(filterFields, bypassFields, remnantFilterFields)));
+ }
+
+ /**
+ * Creates a {@code FilterRequirement} from the requirements of several
+ * table instances.
+ *
+ * <p>Groups without any requirement are discarded: their bypass fields
+ * have nothing left to defuse.
+ *
+ * @param groups Requirements of each table instance
+ */
+ FilterRequirement(Iterable<Group> groups) {
+ final ImmutableList.Builder<Group> groupBuilder = ImmutableList.builder();
+ final ImmutableBitSet.Builder filterFieldBuilder =
ImmutableBitSet.builder();
+ final ImmutableBitSet.Builder bypassFieldBuilder =
ImmutableBitSet.builder();
+ final ImmutableSet.Builder<SqlQualified> remnantFilterFieldBuilder =
ImmutableSet.builder();
+ for (Group group : groups) {
+ if (group.filterFields.isEmpty() && group.remnantFilterFields.isEmpty())
{
+ continue;
+ }
+ groupBuilder.add(group);
+ filterFieldBuilder.addAll(group.filterFields);
+ bypassFieldBuilder.addAll(group.bypassFields);
+ remnantFilterFieldBuilder.addAll(group.remnantFilterFields);
+ }
+ this.groups = groupBuilder.build();
+ this.filterFields = filterFieldBuilder.build();
+ this.bypassFields = bypassFieldBuilder.build();
+ this.remnantFilterFields = remnantFilterFieldBuilder.build();
+ }
+
+ /** Filtering requirements that originate in a single table instance.
+ *
+ * <p>Filtering on any of the {@link #bypassFields} of a group defuses all
+ * of the group's requirements, but not those of other groups. */
+ public static class Group {
+ /** Ordinals (in the row type) of the "must-filter" fields of the table. */
+ public final ImmutableBitSet filterFields;
+
+ /** Ordinals (in the row type) of the "bypass" fields of the table. */
+ public final ImmutableBitSet bypassFields;
+
+ /** Must-filter fields of the table that can no longer be filtered on,
+ * but can still be defused by filtering on one of the
+ * {@link #bypassFields}. */
+ public final ImmutableSet<SqlQualified> remnantFilterFields;
+
+ /**
+ * Creates a {@code Group}.
+ *
+ * @param filterFields Ordinals of the "must-filter" fields
+ * @param bypassFields Ordinals of the "bypass" fields
+ * @param remnantFilterFields Filter fields that can no longer be filtered
+ * on, but can only be defused if a bypass field is filtered on
+ */
+ Group(Iterable<Integer> filterFields, Iterable<Integer> bypassFields,
+ Set<SqlQualified> remnantFilterFields) {
+ this.filterFields = ImmutableBitSet.of(filterFields);
+ this.bypassFields = ImmutableBitSet.of(bypassFields);
+ this.remnantFilterFields = ImmutableSet.copyOf(remnantFilterFields);
+ }
}
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/MatchRecognizeNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/MatchRecognizeNamespace.java
index 91b4e70e39..a9ecba9d69 100644
---
a/core/src/main/java/org/apache/calcite/sql/validate/MatchRecognizeNamespace.java
+++
b/core/src/main/java/org/apache/calcite/sql/validate/MatchRecognizeNamespace.java
@@ -40,6 +40,14 @@ protected MatchRecognizeNamespace(SqlValidatorImpl validator,
@Override public RelDataType validateImpl(RelDataType targetRowType) {
validator.validateMatchRecognize(matchRecognize);
+ // MATCH_RECOGNIZE computes measures over pattern matches of its input's
+ // rows, so a must-filter obligation on the input cannot be re-expressed
+ // in terms of this namespace's columns. Fail closed: require the input
+ // to have discharged its obligations rather than silently dropping them.
+ final SqlValidatorNamespace inputNs =
validator.getNamespace(matchRecognize.getTableRef());
+ if (inputNs != null) {
+ checkFilterRequirementSatisfied(inputNs, matchRecognize.getTableRef());
+ }
return requireNonNull(rowType, "rowType");
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/PivotNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/PivotNamespace.java
index 208dc80f95..5997409580 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/PivotNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/PivotNamespace.java
@@ -37,6 +37,14 @@ protected PivotNamespace(SqlValidatorImpl validator,
SqlPivot pivot,
@Override public RelDataType validateImpl(RelDataType targetRowType) {
validator.validatePivot(pivot);
+ // PIVOT transposes rows and columns, so a must-filter obligation on its
+ // input cannot be re-expressed in terms of this namespace's columns.
+ // Fail closed: require the input to have discharged its obligations
+ // rather than silently dropping them.
+ final SqlValidatorNamespace queryNs = validator.getNamespace(pivot.query);
+ if (queryNs != null) {
+ checkFilterRequirementSatisfied(queryNs, pivot.query);
+ }
return requireNonNull(rowType, "rowType");
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/ProcedureNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/ProcedureNamespace.java
index 9d97a252ef..59c1305b12 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/ProcedureNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/ProcedureNamespace.java
@@ -19,11 +19,13 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlTableFunction;
import org.apache.calcite.sql.type.SqlReturnTypeInference;
import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.util.SqlBasicVisitor;
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -71,6 +73,25 @@ public class ProcedureNamespace extends AbstractNamespace {
}
final SqlReturnTypeInference rowTypeInference =
tableFunction.getRowTypeInference();
+ // A table function transforms the rows of its table arguments (e.g.
+ // "TABLE t" passed to TUMBLE), so a must-filter obligation recorded on
+ // an input cannot be re-expressed in terms of this namespace's columns.
+ // Fail closed: require every table argument to have discharged its
+ // obligations rather than silently dropping them.
+ call.accept(new SqlBasicVisitor<Void>() {
+ @Override public Void visit(SqlCall queryCall) {
+ if (queryCall.getKind().belongsTo(SqlKind.QUERY)) {
+ final SqlValidatorNamespace inputNs =
validator.getNamespace(queryCall);
+ if (inputNs != null) {
+ checkFilterRequirementSatisfied(inputNs, call);
+ // The requirements of the input's own children have already
+ // been aggregated into its namespace; do not descend further.
+ return null;
+ }
+ }
+ return super.visit(queryCall);
+ }
+ });
return requireNonNull(
rowTypeInference.inferReturnType(callBinding),
() -> "got null from inferReturnType for call " +
callBinding.getCall());
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SetopNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/SetopNamespace.java
index 58c381e80c..c479f36f7d 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SetopNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SetopNamespace.java
@@ -24,6 +24,9 @@
import org.checkerframework.checker.nullness.qual.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
import static org.apache.calcite.util.Static.RESOURCE;
import static java.util.Objects.requireNonNull;
@@ -105,13 +108,24 @@ private static SqlMonotonicity combine(@Nullable
SqlMonotonicity m0,
final SqlValidatorScope scope =
requireNonNull(validator.scopes.get(call),
() -> "scope for " + call);
+ final List<FilterRequirement.Group> filterGroups = new ArrayList<>();
for (SqlNode operand : call.getOperandList()) {
if (!operand.isA(SqlKind.QUERY)) {
throw validator.newValidationError(operand,
RESOURCE.needQueryOp(operand.toString()));
}
validator.validateQuery(operand, scope, targetRowType);
+
+ // Propagate the operands' filter requirements to this namespace. The
+ // operands' columns are positionally the columns of the set operation,
+ // so each group keeps its ordinals, and its bypass fields keep
+ // defusing only that group's requirements.
+ final SqlValidatorNamespace operandNamespace =
+ requireNonNull(validator.getNamespace(operand),
+ () -> "namespace for " + operand);
+ filterGroups.addAll(operandNamespace.getFilterRequirement().groups);
}
+ filterRequirement = new FilterRequirement(filterGroups);
return call.getOperator().deriveType(
validator,
scope,
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 1bccf17877..63052a8549 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -1657,33 +1657,13 @@ protected void validateNamespace(final
SqlValidatorNamespace namespace,
RelDataType type = namespace.getType();
if (node == top) {
- final FilterRequirement filterRequirement =
- namespace.getFilterRequirement();
-
- // Either of the following two conditions result in an invalid query:
- // 1) A top-level namespace must not return any must-filter fields.
- // A non-top-level namespace (e.g. a subquery) may return must-filter
- // fields; these are neutralized if the consuming query filters on
them.
- // 2) A top-level namespace must not have any remnant-must-filter
fields.
- // Remnant must filter fields are fields that are not selected and
cannot
- // be defused unless a bypass field defuses it.
- if (!filterRequirement.filterFields.isEmpty()
- || !filterRequirement.remnantFilterFields.isEmpty()) {
- Stream<String> mustFilterStream =
- filterRequirement.filterFields.stream()
- .mapToObj(namespace.getRowType().getFieldNames()::get);
- Stream<String> remnantStream =
- filterRequirement.remnantFilterFields.stream()
- .map(q -> q.suffix().get(0));
-
- // Set of field names, sorted alphabetically for determinism.
- Set<String> fieldNameSet =
- Stream.concat(mustFilterStream, remnantStream)
- .collect(Collectors.toCollection(TreeSet::new));
- throw newValidationError(node,
- RESOURCE.mustFilterFieldsMissing(fieldNameSet.toString()));
+ // The top-level filter check applies only to namespaces that expose
rows to the caller.
+ // A DML statement's target namespace does not: its must-filter fields
describe how the
+ // table is read, not how it is written. The source query of a DML
statement is checked
+ // separately (see validateDmlSourceFilterRequirement).
+ if (!(namespace instanceof DmlNamespace)) {
+ checkFilterRequirementSatisfied(namespace, node);
}
-
if (!config.embeddedQuery()) {
type = SqlTypeUtil.fromMeasure(typeFactory, type);
}
@@ -1692,6 +1672,31 @@ protected void validateNamespace(final
SqlValidatorNamespace namespace,
}
}
+ /**
+ * Checks that {@code namespace} exposes no unsatisfied must-filter
+ * obligations, and throws otherwise.
+ */
+ void checkFilterRequirementSatisfied(SqlValidatorNamespace namespace,
SqlNode node) {
+ namespace.getRowType(); // ensure the source namespace has been validated
+ final FilterRequirement filterRequirement =
namespace.getFilterRequirement();
+ if (filterRequirement.filterFields.isEmpty()
+ && filterRequirement.remnantFilterFields.isEmpty()) {
+ return;
+ }
+ final Stream<String> mustFilterStream =
+ filterRequirement.filterFields.stream()
+ .mapToObj(namespace.getRowType().getFieldNames()::get);
+ final Stream<String> remnantStream =
+ filterRequirement.remnantFilterFields.stream()
+ .map(q -> q.suffix().get(0));
+
+ // Set of field names, sorted alphabetically for determinism.
+ final Set<String> fieldNameSet =
+ Stream.concat(mustFilterStream, remnantStream)
+ .collect(Collectors.toCollection(TreeSet::new));
+ throw newValidationError(node,
RESOURCE.mustFilterFieldsMissing(fieldNameSet.toString()));
+ }
+
@Override public SqlValidatorScope getEmptyScope() {
return new EmptyScope(this);
}
@@ -4832,37 +4837,17 @@ protected void validateSelect(
private static void forEachQualified(SqlNode node, SqlValidatorScope scope,
Consumer<SqlQualified> consumer) {
node.accept(new SqlBasicVisitor<Void>() {
- @Override public Void visit(SqlIdentifier id) {
- final SqlQualified qualified = scope.fullyQualify(id);
- consumer.accept(qualified);
- return null;
+ @Override public Void visit(SqlCall call) {
+ // Do not descend into sub-queries, whose identifiers belong to their
+ // own scope; the outer scope cannot resolve them
+ if (call.getKind().belongsTo(SqlKind.QUERY)) {
+ return null;
+ }
+ return super.visit(call);
}
- });
- }
-
- /** Removes all entries from {@code qualifieds} and
- * {@code remnantMustFilterFields} if {@code node} is a bypassField. */
- private static void purgeForBypassFields(SqlNode node, SqlValidatorScope
scope,
- Set<SqlQualified> qualifieds, Set<SqlQualified> bypassQualifieds,
- Set<SqlQualified> remnantMustFilterFields) {
- node.accept(new SqlBasicVisitor<Void>() {
@Override public Void visit(SqlIdentifier id) {
final SqlQualified qualified = scope.fullyQualify(id);
- if (bypassQualifieds.contains(qualified)) {
- // Clear all the must-filter qualifieds from the same table
identifier
- Collection<SqlQualified> sameIdentifier =
- qualifieds.stream()
- .filter(q -> qualifiedMatchesIdentifier(q, qualified))
- .collect(Collectors.toList());
- sameIdentifier.forEach(qualifieds::remove);
-
- // Clear all the remnant must-filter qualifieds from the same table
identifier
- Collection<SqlQualified> sameIdentifier_ =
- remnantMustFilterFields.stream()
- .filter(q -> qualifiedMatchesIdentifier(q, qualified))
- .collect(Collectors.toList());
- sameIdentifier_.forEach(remnantMustFilterFields::remove);
- }
+ consumer.accept(qualified);
return null;
}
});
@@ -4892,10 +4877,6 @@ private static void toQualifieds(ImmutableBitSet fields,
SqlParserPos.ZERO))));
}
- private static boolean qualifiedMatchesIdentifier(SqlQualified q1,
SqlQualified q2) {
- return q1.identifier.names.get(0).equals(q2.identifier.names.get(0));
- }
-
private void checkRollUpInSelectList(SqlSelect select) {
SqlValidatorScope scope = getSelectScope(select);
final SelectScope selectScope = getRawSelectScopeNonNull(select);
@@ -5374,6 +5355,7 @@ protected void validateQualifyClause(SqlSelect select) {
qualifyNode);
qualifyNode.validate(this, qualifyScope);
+ validateSubQueriesFilterRequirement(qualifyNode);
final RelDataType type = deriveType(qualifyScope, qualifyNode);
if (!SqlTypeUtil.inBooleanFamily(type)) {
@@ -5443,101 +5425,132 @@ protected void
validateMustFilterRequirements(SqlSelect select, SelectNamespace
pipeInputRequirement(ns);
if (select.getFrom() != null) {
final SelectScope fromScope = (SelectScope) getFromScope(select);
- final BitSet projectedNonFilteredBypassField = new BitSet();
- final Set<SqlQualified> qualifieds = new LinkedHashSet<>();
- final Set<SqlQualified> bypassQualifieds = new LinkedHashSet<>();
- final Set<SqlQualified> remnantQualifieds = new LinkedHashSet<>();
- int offset = 0;
- for (ScopeChild child : fromScope.children) {
- final List<String> fieldNames =
- child.namespace.getRowType().getFieldNames();
- final FilterRequirement filterRequirement;
- if (inputRequirement == null) {
- filterRequirement = child.namespace.getFilterRequirement();
- } else {
- // The children's columns are the input's columns, in order (see
- // PipeNamespace#outChildren), so this child's fields start at
offset.
- final int end = offset + fieldNames.size();
- filterRequirement =
- new FilterRequirement(
- inputRequirement.filterFields.get(offset,
end).shift(-offset),
- inputRequirement.bypassFields.get(offset,
end).shift(-offset),
- inputRequirement.remnantFilterFields);
- }
- offset += fieldNames.size();
- toQualifieds(filterRequirement.filterFields, qualifieds, fromScope,
- child, fieldNames);
- toQualifieds(filterRequirement.bypassFields, bypassQualifieds,
- fromScope, child, fieldNames);
- remnantQualifieds.addAll(filterRequirement.remnantFilterFields);
- }
- if (!qualifieds.isEmpty() || !bypassQualifieds.isEmpty()) {
+ // Requirements of each table instance read by this query, in terms of
+ // the columns of the FROM items through which they are read
+ final List<PendingFilterGroup> groups = new ArrayList<>();
+ if (inputRequirement == null) {
+ for (ScopeChild child : fromScope.children) {
+ final List<String> fieldNames =
child.namespace.getRowType().getFieldNames();
+ for (FilterRequirement.Group childGroup :
child.namespace.getFilterRequirement().groups) {
+ final PendingFilterGroup group = new
PendingFilterGroup(childGroup.remnantFilterFields);
+ toQualifieds(childGroup.filterFields, group.qualifieds, fromScope,
child, fieldNames);
+ toQualifieds(childGroup.bypassFields, group.bypassQualifieds,
fromScope, child,
+ fieldNames);
+ groups.add(group);
+ }
+ }
+ } else {
+ // Each group of the input's requirement belongs to one table instance.
+ // The children's columns are the input's columns, in order (see
+ // PipeNamespace#outChildren), so each child's fields start at offset.
+ for (FilterRequirement.Group inputGroup : inputRequirement.groups) {
+ final PendingFilterGroup group = new
PendingFilterGroup(inputGroup.remnantFilterFields);
+ int offset = 0;
+ for (ScopeChild child : fromScope.children) {
+ final List<String> fieldNames =
child.namespace.getRowType().getFieldNames();
+ final int end = offset + fieldNames.size();
+ toQualifieds(inputGroup.filterFields.get(offset,
end).shift(-offset),
+ group.qualifieds, fromScope, child, fieldNames);
+ toQualifieds(inputGroup.bypassFields.get(offset,
end).shift(-offset),
+ group.bypassQualifieds, fromScope, child, fieldNames);
+ offset = end;
+ }
+ groups.add(group);
+ }
+ }
+ if (!groups.isEmpty()) {
+ // A filter on a must-filter field satisfies it. A filter on a bypass
+ // field defuses all the requirements of the table instance it belongs
+ // to, but not those of other table instances, even if they are read
+ // through the same FROM item.
+ final Consumer<SqlQualified> filter = qualified -> {
+ groups.forEach(group -> group.qualifieds.remove(qualified));
+ groups.removeIf(group -> group.bypassQualifieds.contains(qualified));
+ };
if (select.getWhere() != null) {
- forEachQualified(select.getWhere(), getWhereScope(select),
- qualifieds::remove);
- purgeForBypassFields(select.getWhere(), getWhereScope(select),
- qualifieds, bypassQualifieds, remnantQualifieds);
+ forEachQualified(select.getWhere(), getWhereScope(select), filter);
}
if (select.getHaving() != null) {
- forEachQualified(select.getHaving(), getHavingScope(select),
- qualifieds::remove);
- purgeForBypassFields(select.getHaving(), getHavingScope(select),
- qualifieds, bypassQualifieds, remnantQualifieds);
- }
-
- // Each of the must-filter fields identified must be returned as a
- // SELECT item, which is then flagged as must-filter.
- final BitSet mustFilterFields = new BitSet();
- // Each of the bypass fields identified must be returned as a
- // SELECT item, which is then flagged as a bypass field for the
consumer.
- final BitSet mustFilterBypassFields = new BitSet();
+ forEachQualified(select.getHaving(), getHavingScope(select), filter);
+ }
+
final List<SqlNode> expandedSelectItems =
- requireNonNull(fromScope.getExpandedSelectList(),
- "expandedSelectList");
- forEach(expandedSelectItems, (selectItem, i) -> {
- selectItem = stripAs(selectItem);
- if (selectItem instanceof SqlIdentifier) {
- SqlQualified qualified =
- fromScope.fullyQualify((SqlIdentifier) selectItem);
- if (qualifieds.remove(qualified)) {
+ requireNonNull(fromScope.getExpandedSelectList(),
"expandedSelectList");
+ final List<@Nullable SqlQualified> selectQualifieds =
+ Util.transform(expandedSelectItems, selectItem -> {
+ selectItem = stripAs(selectItem);
+ return selectItem instanceof SqlIdentifier
+ ? fromScope.fullyQualify((SqlIdentifier) selectItem)
+ : null;
+ });
+ final List<FilterRequirement.Group> filterGroups = new ArrayList<>();
+ final Set<String> missingFields = new TreeSet<>();
+ for (PendingFilterGroup group : groups) {
+ // Each of the must-filter fields identified must be returned as a
+ // SELECT item, which is then flagged as must-filter.
+ final BitSet mustFilterFields = new BitSet();
+ // Each of the bypass fields identified must be returned as a
+ // SELECT item, which is then flagged as a bypass field for the
+ // consumer.
+ final BitSet mustFilterBypassFields = new BitSet();
+ forEach(selectQualifieds, (qualified, i) -> {
+ if (qualified == null) {
+ return;
+ }
+ if (group.qualifieds.remove(qualified)) {
// SELECT item #i referenced a must-filter column that was not
// filtered in the WHERE or HAVING. It becomes a must-filter
// column for our consumer.
mustFilterFields.set(i);
}
- if (bypassQualifieds.remove(qualified)) {
+ if (group.bypassQualifieds.remove(qualified)) {
// SELECT item #i referenced a bypass column that was not
filtered
// in the WHERE or HAVING. It becomes a bypass column for our
// consumer.
mustFilterBypassFields.set(i);
- projectedNonFilteredBypassField.set(0);
}
- }
- });
+ });
- // If there are must-filter fields that are not in the SELECT clause
and
- // there were no bypass-fields on this table, this is an error.
- if (!qualifieds.isEmpty() && !projectedNonFilteredBypassField.get(0)) {
+ // If there are must-filter fields that are not in the SELECT clause
+ // and there were no bypass-fields on this table, this is an error.
+ if (!group.qualifieds.isEmpty() && mustFilterBypassFields.isEmpty())
{
+ group.qualifieds.forEach(q ->
missingFields.add(q.suffix().get(0)));
+ }
+ // Remaining must-filter fields can be defused by a bypass-field,
+ // so we pass this to the consumer.
+ final Set<SqlQualified> remnantMustFilterFields =
+ new LinkedHashSet<>(group.remnantQualifieds);
+ remnantMustFilterFields.addAll(group.qualifieds);
+ filterGroups.add(
+ new FilterRequirement.Group(
+ ImmutableBitSet.fromBitSet(mustFilterFields),
+ ImmutableBitSet.fromBitSet(mustFilterBypassFields),
+ remnantMustFilterFields));
+ }
+ if (!missingFields.isEmpty()) {
throw newValidationError(select,
- RESOURCE.mustFilterFieldsMissing(
- qualifieds.stream()
- .map(q -> q.suffix().get(0))
- .collect(Collectors.toCollection(TreeSet::new))
- .toString()));
+ RESOURCE.mustFilterFieldsMissing(missingFields.toString()));
}
- // Remaining must-filter fields can be defused by a bypass-field,
- // so we pass this to the consumer.
- ImmutableSet<SqlQualified> remnantMustFilterFields =
- Stream.of(remnantQualifieds, qualifieds)
- .flatMap(Set::stream).collect(ImmutableSet.toImmutableSet());
- ns.filterRequirement =
- new FilterRequirement(ImmutableBitSet.fromBitSet(mustFilterFields),
- ImmutableBitSet.fromBitSet(mustFilterBypassFields),
- remnantMustFilterFields);
+ ns.filterRequirement = new FilterRequirement(filterGroups);
}
}
}
+ /** Filter requirements of a table instance read by a SELECT, in terms of
+ * the columns of the FROM items, while they are being defused. */
+ private static class PendingFilterGroup {
+ /** Must-filter fields that have not been filtered yet. */
+ final Set<SqlQualified> qualifieds = new LinkedHashSet<>();
+ /** Bypass fields that can defuse this group's requirements. */
+ final Set<SqlQualified> bypassQualifieds = new LinkedHashSet<>();
+ /** Must-filter fields that can only be defused by a bypass field. */
+ final Set<SqlQualified> remnantQualifieds;
+
+ PendingFilterGroup(Set<SqlQualified> remnantQualifieds) {
+ this.remnantQualifieds = remnantQualifieds;
+ }
+ }
+
@Override public void validateWith(SqlWith with, SqlValidatorScope scope) {
final SqlValidatorNamespace namespace = getNamespaceOrThrow(with);
validateNamespace(namespace, unknownType);
@@ -6118,6 +6131,10 @@ protected void validateWhereOrOn(
if (!isReturnBooleanType(type)) {
throw newValidationError(condition, RESOURCE.condMustBeBoolean(clause));
}
+
+ // Sub-queries used as expressions cannot forward must-filter
+ // requirements to the enclosing query; enforce them here (fail-closed).
+ validateSubQueriesFilterRequirement(condition);
}
private static boolean isReturnBooleanType(RelDataType relDataType) {
@@ -6160,6 +6177,10 @@ protected void validateHavingClause(SqlSelect select) {
if (!SqlTypeUtil.inBooleanFamily(type)) {
throw newValidationError(having, RESOURCE.havingMustBeBoolean());
}
+
+ // Sub-queries used as expressions cannot forward must-filter
+ // requirements to the enclosing query; enforce them here (fail-closed).
+ validateSubQueriesFilterRequirement(having);
}
/** Validates that SELECT items do not qualify common columns
@@ -6605,6 +6626,30 @@ private void validateExpr(SqlNode expr,
SqlValidatorScope scope) {
// Perform any validation specific to the scope. For example, an
// aggregating scope requires that expressions are valid aggregations.
scope.validateExpr(expr);
+
+ // Sub-queries used as expressions cannot forward must-filter
+ // requirements to the enclosing query; enforce them here (fail-closed).
+ validateSubQueriesFilterRequirement(expr);
+ }
+
+ /** Validates that the sub-queries inside an expression have no pending
+ * "must-filter" obligations.
+ *
+ * @param expr Expression, possibly containing sub-queries
+ */
+ void validateSubQueriesFilterRequirement(SqlNode expr) {
+ expr.accept(new SqlBasicVisitor<Void>() {
+ @Override public Void visit(SqlCall call) {
+ if (call.getKind().belongsTo(SqlKind.QUERY)) {
+ final SqlValidatorNamespace ns = getNamespace(call);
+ if (ns != null) {
+ checkFilterRequirementSatisfied(ns, call);
+ return null;
+ }
+ }
+ return super.visit(call);
+ }
+ });
}
/**
@@ -6707,6 +6752,12 @@ protected RelDataType createTargetRowType(
return typeFactory.createStructType(fields);
}
+ /** Throws if {@code source}, the read side of a DML statement, has
+ * outstanding filter requirements. */
+ private void validateDmlSourceFilterRequirement(SqlNode source) {
+ checkFilterRequirementSatisfied(getNamespaceOrThrow(source), source);
+ }
+
@Override public void validateInsert(SqlInsert insert) {
final SqlValidatorNamespace targetNamespace = getNamespaceOrThrow(insert);
validateNamespace(targetNamespace, unknownType);
@@ -6737,6 +6788,8 @@ protected RelDataType createTargetRowType(
validateQuery(source, scope, targetRowType);
}
+ validateDmlSourceFilterRequirement(source);
+
// REVIEW jvs 4-Dec-2008: In FRG-365, this namespace row type is
// discarding the type inferred by inferUnknownTypes (which was invoked
// from validateSelect above). It would be better if that information
@@ -7124,6 +7177,7 @@ private static SqlNode getNthExpr(SqlNode query, int
ordinal, int sourceCount) {
@Override public void validateDelete(SqlDelete call) {
final SqlSelect sqlSelect = SqlNonNullableAccessors.getSourceSelect(call);
validateSelect(sqlSelect, unknownType);
+ validateDmlSourceFilterRequirement(sqlSelect);
final SqlValidatorNamespace targetNamespace = getNamespaceOrThrow(call);
validateNamespace(targetNamespace, unknownType);
@@ -7149,6 +7203,7 @@ private static SqlNode getNthExpr(SqlNode query, int
ordinal, int sourceCount) {
final SqlSelect select = SqlNonNullableAccessors.getSourceSelect(call);
validateSelect(select, targetRowType);
+ validateDmlSourceFilterRequirement(select);
final RelDataType sourceRowType = getValidatedNodeType(select);
checkTypeAssignment(scopes.get(select), table, sourceRowType,
targetRowType,
@@ -7206,6 +7261,7 @@ private static SqlNode getNthExpr(SqlNode query, int
ordinal, int sourceCount) {
}
validateSelect(sqlSelect, targetRowType);
+ validateDmlSourceFilterRequirement(sqlSelect);
SqlUpdate updateCallAfterValidate = call.getUpdateCall();
if (updateCallAfterValidate != null) {
@@ -7338,6 +7394,7 @@ protected void validateValues(
for (SqlNode operand : operands) {
operand.validate(this, scope);
+ validateSubQueriesFilterRequirement(operand);
}
// validate that all row types have the same number of columns
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/UnnestNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/UnnestNamespace.java
index 34b6be2a30..1735324f22 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/UnnestNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/UnnestNamespace.java
@@ -95,6 +95,9 @@ class UnnestNamespace extends AbstractNamespace {
@Override protected RelDataType validateImpl(RelDataType targetRowType) {
// Validate the call and its arguments, and infer the return type.
validator.validateCall(unnest, scope);
+ // Sub-queries used as UNNEST operands (e.g. "UNNEST(ARRAY(SELECT ...))")
+ // cannot forward must-filter requirements to the enclosing query, enforce
them here
+ validator.validateSubQueriesFilterRequirement(unnest);
RelDataType type =
unnest.getOperator().validateOperands(validator, scope, unnest);
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/UnpivotNamespace.java
b/core/src/main/java/org/apache/calcite/sql/validate/UnpivotNamespace.java
index 5c699618b6..6959fcd62d 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/UnpivotNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/UnpivotNamespace.java
@@ -37,6 +37,14 @@ protected UnpivotNamespace(SqlValidatorImpl validator,
SqlUnpivot unpivot,
@Override public RelDataType validateImpl(RelDataType targetRowType) {
validator.validateUnpivot(unpivot);
+ // UNPIVOT transposes columns and rows, so a must-filter obligation on
+ // its input cannot be re-expressed in terms of this namespace's columns.
+ // Fail closed: require the input to have discharged its obligations
+ // rather than silently dropping them.
+ final SqlValidatorNamespace queryNs =
validator.getNamespace(unpivot.query);
+ if (queryNs != null) {
+ checkFilterRequirementSatisfied(queryNs, unpivot.query);
+ }
return requireNonNull(rowType, "rowType");
}
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 313e7e5703..1453459c22 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -15178,6 +15178,43 @@ private SqlCall checkArrayOperandType(String sql,
String expectedArrayType,
assertThat(resultType, hasToString("INTEGER"));
}
+ /**
+ * Tests validation of must-filter columns when a table is aliased with a
+ * column list, {@code AS t (c1, ..., cN)}, which is implemented by
+ * {@link org.apache.calcite.sql.validate.AliasNamespace}. The renaming is
+ * positional, so filter requirements must be preserved ordinal for
+ * ordinal.
+ */
+ @Test void testMustFilterColumnsWithAliasColumnList() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ // Aliasing with a column list must not drop the must-filter fields
+ // ("a" renames EMPNO, "c" renames JOB)
+ fixture.withSql("^select * from emp as t(a, b, c, d, e, f, g, h, i)^")
+ .fails(missingFilters("A", "C"));
+ fixture.withSql("^select a, c from emp as t(a, b, c, d, e, f, g, h, i)^")
+ .fails(missingFilters("A", "C"));
+
+ // Filters on the renamed columns are filters on the underlying
must-filter columns
+ fixture.withSql("select * from emp as t(a, b, c, d, e, f, g, h, i)\n"
+ + "where a = 1 and c = 'doctor'")
+ .ok();
+
+ // "b" renames the bypass field ENAME; filtering on it defuses the
requirement
+ fixture.withSql("select * from emp as t(a, b, c, d, e, f, g, h, i)\n"
+ + "where b = '1'")
+ .ok();
+
+ // An enclosing query can still defuse the propagated requirement
+ fixture.withSql("select * from (\n"
+ + " select * from emp as t(a, b, c, d, e, f, g, h, i))\n"
+ + "where a = 1 and c = 'doctor'")
+ .ok();
+ }
+
/**
* Tests validation of must-filter columns.
*
@@ -15391,6 +15428,95 @@ private SqlCall checkArrayOperandType(String sql,
String expectedArrayType,
.fails(missingFilters("JOB"));
}
+ /**
+ * Tests validation of must-filter columns in set operations.
+ */
+ @Test void testMustFilterColumnsInSetOp() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ final List<String> ops = Arrays.asList("union", "union all", "intersect",
"except");
+ for (String op : ops) {
+ // Wrapping a query in a set operation must not drop the operands'
+ // must-filter requirements
+ fixture.withSql("^select empno, job from emp\n"
+ + op + "\n"
+ + "select empno, job from emp^")
+ .fails(missingFilters("EMPNO", "JOB"));
+
+ // Valid if every operand applies the filters itself
+ fixture.withSql("select empno, job from emp\n"
+ + "where empno = 1 and job = 'doctor'\n"
+ + op + "\n"
+ + "select empno, job from emp\n"
+ + "where empno = 2 and job = 'undertaker'")
+ .ok();
+
+ // Valid because an enclosing query can still defuse the propagated
+ // requirement by filtering on the set operation's columns
+ fixture.withSql("select * from (\n"
+ + "select empno, job from emp\n"
+ + op + "\n"
+ + "select empno, job from emp)\n"
+ + "where empno = 1 and job = 'doctor'")
+ .ok();
+
+ // Valid because ENAME is a bypass field of EMP; filtering on it defuses
+ // the requirement inside each operand
+ fixture.withSql("select empno, job from emp where ename = '1'\n"
+ + op + "\n"
+ + "select empno, job from emp where ename = '2'")
+ .ok();
+
+ // Not valid because bypass field ENAME is not filtered in all operands
+ fixture.withSql("^select empno, job from emp where ename = '1'\n"
+ + op + "\n"
+ + "select empno, job from emp^")
+ .fails(missingFilters("EMPNO", "JOB"));
+ }
+ }
+
+ /**
+ * Tests validation of must-filter columns in set operations where one of
+ * the operands has no must-filter requirements. Such an operand must not
+ * prevent the other operands' remnant requirements from being defused by
+ * filtering a bypass field on the set operation's output.
+ */
+ @Test void testMustFilterColumnsInSetOpWithUnrestrictedOperand() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ final List<String> ops = Arrays.asList("union", "union all", "intersect",
"except");
+ for (String op : ops) {
+ // Not valid because the remnant requirement on EMP.JOB is not defused
+ fixture.withSql("^select * from (\n"
+ + "select ename from emp where empno = 1\n"
+ + op + "\n"
+ + "select name from bonus) as x^")
+ .fails(missingFilters("JOB"));
+
+ // Valid because ENAME is a bypass field of the only operand that has
+ // requirements (BONUS has none), so filtering on it defuses the remnant
+ // requirement on EMP.JOB
+ fixture.withSql("select * from (\n"
+ + "select ename from emp where empno = 1\n"
+ + op + "\n"
+ + "select name from bonus) as x\n"
+ + "where x.ename = 'doctor'")
+ .ok();
+ fixture.withSql("select * from (\n"
+ + "select name from bonus\n"
+ + op + "\n"
+ + "select ename from emp where empno = 1) as x\n"
+ + "where x.name = 'doctor'")
+ .ok();
+ }
+ }
+
/**
* Tests validation of must-filter columns with the inclusion of bypass
fields.
*
@@ -15744,6 +15870,152 @@ private SqlCall checkArrayOperandType(String sql,
String expectedArrayType,
.fails(missingFilters("JOB"));
}
+ /**
+ * Tests that must-filter columns cannot be bypassed by wrapping the
+ * protected table in a construct that transforms its rows (PIVOT,
+ * UNPIVOT, MATCH_RECOGNIZE or a table function) for which the filter
+ * requirement cannot be re-expressed in terms of the output columns.
+ * Validation fails closed: the filters must be applied within the input
+ * query itself.
+ */
+ @Test void testMustFilterColumnsFailClosedForRowTransforms() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ // PIVOT
+ fixture.withSql("select * from ^emp^\n"
+ + "pivot (sum(sal) as ss for job in ('CLERK' as c))")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("select * from\n"
+ + " (select * from emp where empno = 1 and job = 'doctor')\n"
+ + "pivot (sum(sal) as ss for deptno in (10 as d10))")
+ .ok();
+
+ // UNPIVOT
+ fixture.withSql("select * from ^emp^\n"
+ + "unpivot (remuneration\n"
+ + " for remuneration_type in (comm as 'commission', sal as
'salary'))")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("select * from\n"
+ + " (select * from emp where empno = 1 and job = 'doctor')\n"
+ + "unpivot (remuneration\n"
+ + " for remuneration_type in (comm as 'commission', sal as
'salary'))")
+ .ok();
+
+ // MATCH_RECOGNIZE
+ fixture.withSql("select * from ^emp^\n"
+ + "match_recognize (\n"
+ + " measures A.empno as e\n"
+ + " pattern (A)\n"
+ + " define A as A.empno > 0\n"
+ + ") as t")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("select * from\n"
+ + " (select * from emp where empno = 1 and job = 'doctor')\n"
+ + "match_recognize (\n"
+ + " measures A.empno as e\n"
+ + " pattern (A)\n"
+ + " define A as A.empno > 0\n"
+ + ") as t")
+ .ok();
+
+ // Table functions pass their input tables' rows through (e.g. TUMBLE)
+ fixture.withSql("select * from table(\n"
+ + "^tumble(table emp, descriptor(hiredate), interval '2' hour)^)")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("select * from table(\n"
+ + "tumble((select * from emp where empno = 1 and job =
'doctor'),\n"
+ + " descriptor(hiredate), interval '2' hour))")
+ .ok();
+ }
+
+ /**
+ * Tests that a must-filter obligation carried out of a subquery (a
+ * "remnant" obligation) can only be defused by filtering a bypass field of
+ * the table instance that produced it, not by filtering a bypass field of
+ * another instance of the same table that happens to carry the same alias.
+ */
+ @Test void testMustFilterRemnantMatchesTableInstance() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ // The subquery leaves a remnant obligation on EMP.JOB (JOB is neither
+ // filtered nor selected, but the bypass field ENAME is selected);
+ // filtering ENAME on a second join instance of EMP, aliased "EMP" just
+ // like the table inside the subquery, must not defuse the subquery's
+ // remnant obligation: the filter constrains none of the subquery's rows
+ fixture.withSql("^select x.ename\n"
+ + "from (select ename from emp where empno = 1) as x\n"
+ + "join emp as emp on true\n"
+ + "where emp.ename = 'doctor'^")
+ .fails(missingFilters("JOB"));
+
+ // Filtering the bypass field of the subquery itself defuses the remnant
+ fixture.withSql("select x.ename\n"
+ + "from (select ename from emp where empno = 1) as x\n"
+ + "where x.ename = 'doctor'")
+ .ok();
+
+ // A query whose only outstanding obligation is a remnant imported from a
+ // subquery must propagate it to the top-level check
+ fixture.withSql("^select * from (\n"
+ + " select sal from (\n"
+ + " select ename, sal from emp where empno = 1))^")
+ .fails(missingFilters("JOB"));
+ }
+
+ /**
+ * Tests that, when a subquery joins several tables that each leave a
+ * remnant must-filter obligation, filtering the bypass field of one table
+ * defuses only that table's remnant obligation, not the other tables'.
+ */
+ @Test void testMustFilterRemnantMatchesSourceTable() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ // The subquery leaves a remnant obligation on EMP.JOB (defusable via the
+ // bypass field ENAME) and another on DEPT.NAME (defusable via the bypass
+ // field DEPTNO). Filtering ENAME must not defuse DEPT.NAME.
+ fixture.withSql("^select * from (\n"
+ + " select e.ename, d.deptno from emp as e, dept as d\n"
+ + " where e.empno = 1) as x\n"
+ + "where x.ename = 'doctor'^")
+ .fails(missingFilters("NAME"));
+
+ // Likewise, filtering DEPTNO must not defuse EMP.JOB
+ fixture.withSql("^select * from (\n"
+ + " select e.ename, d.deptno from emp as e, dept as d\n"
+ + " where e.empno = 1) as x\n"
+ + "where x.deptno = 10^")
+ .fails(missingFilters("JOB"));
+
+ // Valid because both bypass fields are filtered
+ fixture.withSql("select * from (\n"
+ + " select e.ename, d.deptno from emp as e, dept as d\n"
+ + " where e.empno = 1) as x\n"
+ + "where x.ename = 'doctor' and x.deptno = 10")
+ .ok();
+
+ // DEPT.NAME is still a must-filter field of the subquery; filtering the
+ // bypass field ENAME of EMP must not defuse it
+ fixture.withSql("^select * from (\n"
+ + " select e.ename, d.name from emp as e, dept as d\n"
+ + " where e.empno = 1 and e.job = 'a') as x\n"
+ + "where x.ename = 'doctor'^")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("select * from (\n"
+ + " select e.ename, d.name from emp as e, dept as d\n"
+ + " where e.empno = 1 and e.job = 'a') as x\n"
+ + "where x.name = 'doctor'")
+ .ok();
+ }
+
@Test void testMustFilterColumnsInPipeQuery() {
final SqlValidatorFixture fixture = fixture()
.withConformance(SqlConformanceEnum.LENIENT)
@@ -15771,6 +16043,56 @@ private SqlCall checkArrayOperandType(String sql,
String expectedArrayType,
+ "^|> where empno = 1 and job = 'doctor'^")
.fails(missingFilters("NAME"));
+ // In a pipeline of several steps, each step owes only what the previous
+ // steps have not defused.
+ fixture.withSql("from emp\n"
+ + "|> where empno = 1 and job = 'doctor'\n"
+ + "|> where sal > 1000")
+ .ok();
+ fixture.withSql("from emp\n"
+ + "|> where empno = 1\n"
+ + "|> where job = 'doctor'")
+ .ok();
+ fixture.withSql("from emp\n"
+ + "|> where empno = 1\n"
+ + "^|> where sal > 1000^")
+ .fails(missingFilters("JOB"));
+
+ // A bypass field defuses the requirements in the step that filters it,
+ // and in all later steps.
+ fixture.withSql("from emp\n"
+ + "|> where ename = 'x'\n"
+ + "|> where sal > 1000")
+ .ok();
+ fixture.withSql("from emp\n"
+ + "|> where empno = 1\n"
+ + "|> where ename = 'x'")
+ .ok();
+
+ // With two tables and several steps, each table's fields are found at its
+ // own offset in each step's input, and a bypass field defuses only the
+ // requirements of its own table.
+ fixture.withSql("from emp, dept\n"
+ + "|> where empno = 1 and job = 'doctor'\n"
+ + "|> where name = 'x'")
+ .ok();
+ fixture.withSql("from emp, dept\n"
+ + "|> where empno = 1 and job = 'doctor'\n"
+ + "^|> where sal > 1000^")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("from emp, dept\n"
+ + "|> where name = 'x'\n"
+ + "^|> where empno = 1^")
+ .fails(missingFilters("JOB"));
+ fixture.withSql("from emp, dept\n"
+ + "|> where ename = 'x'\n"
+ + "|> where name = 'y'")
+ .ok();
+ fixture.withSql("from emp, dept\n"
+ + "|> where ename = 'x'\n"
+ + "^|> where sal > 1000^")
+ .fails(missingFilters("NAME"));
+
// A pipeline in a sub-query is defused by the enclosing query, and
// enforced at the top if it is not.
fixture.withSql("select * from (from emp |> where sal > 1000)\n"
@@ -15787,6 +16109,157 @@ private static String missingFilters(String... args) {
+ "\\]";
}
+ @Test void testMustFilterColumnsInDml() {
+ final SqlValidatorFixture fixture = fixture()
+ .withParserConfig(c -> c.withQuoting(Quoting.BACK_TICK))
+ .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY))
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ // --- INSERT ---
+ fixture.withSql("insert into dept ^select empno, job from emp^")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("insert into dept\n"
+ + "^select empno, ename from emp where empno = 1^")
+ .fails(missingFilters("JOB"));
+ fixture.withSql("insert into dept\n"
+ + "select empno, job from emp\n"
+ + "where empno = 1 and job = 'doctor'")
+ .ok();
+ fixture.withSql("insert into dept\n"
+ + "select empno, job from emp\n"
+ + "where ename = 'doctor'")
+ .ok();
+
+ // --- UPDATE ---
+ fixture.withSql("update emp set sal = 100")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("update emp set sal = 100 where deptno = 10")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("update emp set sal = 100 where empno = 1")
+ .fails(missingFilters("JOB"));
+ fixture.withSql("update emp set sal = 100\n"
+ + "where empno = 1 and job = 'doctor'")
+ .ok();
+ fixture.withSql("update emp set sal = 100 where ename = 'foo'")
+ .ok();
+
+ // --- DELETE ---
+ fixture.withSql("delete from emp")
+ .fails(missingFilters("EMPNO", "JOB"));
+ fixture.withSql("delete from emp where empno = 1")
+ .fails(missingFilters("JOB"));
+ fixture.withSql("delete from emp\n"
+ + "where empno = 1 and job = 'doctor'")
+ .ok();
+ fixture.withSql("delete from emp\n"
+ + "where ename = 'doctor'")
+ .ok();
+
+ // --- MERGE ---
+ fixture.withSql("merge into dept d\n"
+ + "using emp e on d.deptno = e.deptno\n"
+ + "when matched then update set name = e.ename\n"
+ + "when not matched then insert (deptno, name) values (e.deptno,
e.ename)")
+ .fails(missingFilters("EMPNO", "JOB", "NAME"));
+ fixture.withSql("merge into dept d\n"
+ + "using (select * from emp where empno = 1 and job = 'doctor')
e\n"
+ + "on d.deptno = e.deptno\n"
+ + "when matched then update set name = e.ename\n"
+ + "when not matched then insert (deptno, name) values (e.deptno,
e.ename)")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("merge into bonus b\n"
+ + "using (select * from emp where empno = 1 and job = 'doctor')
e\n"
+ + "on b.empno = e.empno\n"
+ + "when matched then update set name = e.ename\n"
+ + "when not matched then insert (empno, name) values (e.empno,
e.ename)")
+ .ok();
+ }
+
+ @Test void testMustFilterColumnsInExpressionSubQueries() {
+ final SqlValidatorFixture fixture = fixture()
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ // Scalar sub-query in the SELECT list
+ fixture.withSql("select ^(select name from dept)^\n"
+ + "from emp\n"
+ + "where empno = 1 and job = 'doctor'")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("select (select name from dept where name =
'accounting')\n"
+ + "from emp\n"
+ + "where empno = 1 and job = 'doctor'")
+ .ok();
+
+ // Valid because DEPTNO is a bypass field of DEPT
+ fixture.withSql("select (select name from dept where deptno = 1)\n"
+ + "from emp\n"
+ + "where empno = 1 and job = 'doctor'")
+ .ok();
+
+ // Sub-query as an IN operand in the WHERE clause
+ fixture.withSql("select empno, job from emp\n"
+ + "where empno in (^select deptno from dept^) and job = 'doctor'")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("select empno, job from emp\n"
+ + "where empno in (select deptno from dept where name =
'accounting')\n"
+ + "and job = 'doctor'")
+ .ok();
+
+ // Sub-query as an EXISTS operand in the WHERE clause
+ fixture.withSql("select empno, job from emp\n"
+ + "where empno = 1 and exists ^(select name from dept)^\n"
+ + "and job = 'doctor'")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("select empno, job from emp\n"
+ + "where empno = 1\n"
+ + "and exists (select name from dept where name = 'accounting')\n"
+ + "and job = 'doctor'")
+ .ok();
+
+ // Sub-query in VALUES
+ fixture.withSql("insert into bonus (empno, name)\n"
+ + "values (1, ^(select name from dept)^)")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("insert into bonus (empno, name)\n"
+ + "values (1, (select name from dept where name = 'accounting'))")
+ .ok();
+
+ // Sub-query in QUALIFY
+ fixture.withSql("select empno, ename, sum(deptno) over myWindow as
sumDeptNo\n"
+ + "from emp WHERE empno > 1 and job = 'doctor'\n"
+ + "window myWindow as (partition by ename order by empno)\n"
+ + "qualify sumDeptNo in (^select deptno from dept^)")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("select empno, ename, sum(deptno) over myWindow as
sumDeptNo\n"
+ + "from emp WHERE empno > 1 and job = 'doctor'\n"
+ + "window myWindow as (partition by ename order by empno)\n"
+ + "qualify sumDeptNo in (select deptno from dept where name =
'accounting')")
+ .ok();
+ }
+
+ /**
+ * Tests that a must-filter requirement inside a sub-query used as the
+ * operand of UNNEST (via an ARRAY query constructor) is enforced, rather
+ * than being silently dropped.
+ */
+ @Test void testMustFilterColumnsInUnnestSubQuery() {
+ final SqlValidatorFixture fixture = fixture()
+ .withCatalogReader(MustFilterMockCatalogReader::create);
+
+ fixture.withSql("select * from unnest(array(^select name from dept^))")
+ .fails(missingFilters("NAME"));
+ fixture.withSql("select * from bonus as b\n"
+ + "cross join unnest(array(^select name from dept^))")
+ .fails(missingFilters("NAME"));
+
+ fixture.withSql("select * from unnest(\n"
+ + "array(select name from dept where name = 'accounting'))")
+ .ok();
+ // Valid because DEPTNO is a bypass field of DEPT
+ fixture.withSql("select * from unnest(\n"
+ + "array(select name from dept where deptno = 10))")
+ .ok();
+ }
+
@Test void testAccessingNestedFieldsOfNullableRecord() {
sql("select ROW_COLUMN_ARRAY[0].NOT_NULL_FIELD from NULLABLEROWS.NR_T1")
.withExtendedCatalog()
diff --git
a/testkit/src/main/java/org/apache/calcite/test/catalog/MustFilterMockCatalogReader.java
b/testkit/src/main/java/org/apache/calcite/test/catalog/MustFilterMockCatalogReader.java
index ab1addd943..cb66774ea8 100644
---
a/testkit/src/main/java/org/apache/calcite/test/catalog/MustFilterMockCatalogReader.java
+++
b/testkit/src/main/java/org/apache/calcite/test/catalog/MustFilterMockCatalogReader.java
@@ -84,6 +84,15 @@ public static SqlValidatorCatalogReader
create(RelDataTypeFactory typeFactory,
deptTable.addColumn("DEPTNO", integerType, true);
deptTable.addColumn("NAME", varcharType);
registerTable(deptTable);
+
+ // Register "BONUS" table. No must-filter or bypass fields.
+ MustFilterMockTable bonusTable =
+ MustFilterMockTable.create(this, salesSchema, "BONUS",
+ false, 14, null, NullInitializerExpressionFactory.INSTANCE,
+ false, ImmutableMap.of(), ImmutableList.of());
+ bonusTable.addColumn("EMPNO", integerType, true);
+ bonusTable.addColumn("NAME", varcharType);
+ registerTable(bonusTable);
return this;
}
}