This is an automated email from the ASF dual-hosted git repository. mhubail pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 03fda280bbf6ffca60224ce801da4fafd28b389c Author: Ali Alsuliman <[email protected]> AuthorDate: Tue Apr 8 18:01:42 2025 -0700 [ASTERIXDB-3595][COMP] Ensure path node exists when handling path - user model changes: no - storage format changes: no - interface changes: no Details: When pushing a filter and handling field access paths, ensure the path node exists. Ext-ref: MB-66204 Change-Id: Ie11d88c6ace3dcc08744d3b04add679a6206a41c Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19635 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Ritik Raj <[email protected]> --- .../processor/AbstractFilterPushdownProcessor.java | 22 ++++++++++++- .../processor/ColumnFilterPushdownProcessor.java | 11 +++++-- .../ColumnRangeFilterPushdownProcessor.java | 11 ++++--- .../DeltaTableFilterPushdownProcessor.java | 6 ++-- .../ExternalDatasetFilterPushdownProcessor.java | 6 ++-- .../processor/ParquetFilterPushdownProcessor.java | 8 ++--- .../queries/column-filter/ASTERIXDB-3595.001.sqlpp | 34 +++++++++++++++++++ .../results/column-filter/ASTERIXDB-3595.001.plan | 38 ++++++++++++++++++++++ 8 files changed, 117 insertions(+), 19 deletions(-) diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/AbstractFilterPushdownProcessor.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/AbstractFilterPushdownProcessor.java index 7d4f7a55df..92436c7b1c 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/AbstractFilterPushdownProcessor.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/AbstractFilterPushdownProcessor.java @@ -40,6 +40,7 @@ import org.apache.asterix.optimizer.rules.pushdown.PushdownContext; import org.apache.asterix.optimizer.rules.pushdown.descriptor.DefineDescriptor; import org.apache.asterix.optimizer.rules.pushdown.descriptor.ScanDefineDescriptor; import org.apache.asterix.optimizer.rules.pushdown.descriptor.UseDescriptor; +import org.apache.asterix.optimizer.rules.pushdown.schema.IExpectedSchemaNode; import org.apache.asterix.optimizer.rules.pushdown.visitor.FilterExpressionInlineVisitor; import org.apache.commons.lang3.mutable.Mutable; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; @@ -133,7 +134,26 @@ abstract class AbstractFilterPushdownProcessor extends AbstractPushdownProcessor * @param expression path expression * @return true if the pushdown should continue, false otherwise */ - protected abstract boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException; + protected final boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException { + IExpectedSchemaNode node = getPathNode(expression); + if (node == null) { + return false; + } + return handlePath(expression, node); + } + + /** + * Handle a value access path expression + * + * @param expression path expression + * @param node expected schema node (never null) + * @return true if the pushdown should continue, false otherwise + */ + protected abstract boolean handlePath(AbstractFunctionCallExpression expression, IExpectedSchemaNode node) + throws AlgebricksException; + + protected abstract IExpectedSchemaNode getPathNode(AbstractFunctionCallExpression expression) + throws AlgebricksException; /** * Put the filter expression to data-scan diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnFilterPushdownProcessor.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnFilterPushdownProcessor.java index 0d79767ea3..497e751136 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnFilterPushdownProcessor.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnFilterPushdownProcessor.java @@ -117,9 +117,9 @@ public class ColumnFilterPushdownProcessor extends AbstractFilterPushdownProcess } @Override - protected boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException { - IExpectedSchemaNode node = expression.accept(exprToNodeVisitor, null); - if (node == null || node.getType() != ExpectedSchemaNodeType.ANY) { + protected boolean handlePath(AbstractFunctionCallExpression expression, IExpectedSchemaNode node) + throws AlgebricksException { + if (node.getType() != ExpectedSchemaNodeType.ANY) { return false; } paths.put(expression, pathBuilderVisitor.buildPath((AnyExpectedSchemaNode) node)); @@ -154,6 +154,11 @@ public class ColumnFilterPushdownProcessor extends AbstractFilterPushdownProcess scanDefineDescriptor.getFilterPaths().putAll(paths); } + @Override + protected IExpectedSchemaNode getPathNode(AbstractFunctionCallExpression expression) throws AlgebricksException { + return expression.accept(exprToNodeVisitor, null); + } + protected final AbstractFunctionCallExpression andExpression(ILogicalExpression filterExpr, ILogicalExpression inlinedExpr) { AbstractFunctionCallExpression funcExpr = (AbstractFunctionCallExpression) filterExpr; diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnRangeFilterPushdownProcessor.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnRangeFilterPushdownProcessor.java index 75a7608403..030fb6ee7c 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnRangeFilterPushdownProcessor.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnRangeFilterPushdownProcessor.java @@ -102,16 +102,19 @@ public class ColumnRangeFilterPushdownProcessor extends ColumnFilterPushdownProc } @Override - protected boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException { + protected boolean handlePath(AbstractFunctionCallExpression expression, IExpectedSchemaNode node) + throws AlgebricksException { // This means we got something like WHERE $r.getField("isVerified") -- where isVerified is a boolean field. - AnyExpectedSchemaNode node = getNode(expression); + if (node.getType() != ExpectedSchemaNodeType.ANY) { + return false; + } IAObject constantValue = ABoolean.TRUE; String functionName = expression.getFunctionIdentifier().getName(); SourceLocation sourceLocation = expression.getSourceLocation(); FunctionCallInformation functionCallInfo = new FunctionCallInformation(functionName, sourceLocation, ProjectionFiltrationWarningFactoryProvider.getIncomparableTypesFactory(false)); - ARecordType path = - pathBuilderVisitor.buildPath(node, constantValue.getType(), sourceInformationMap, functionCallInfo); + ARecordType path = pathBuilderVisitor.buildPath((AnyExpectedSchemaNode) node, constantValue.getType(), + sourceInformationMap, functionCallInfo); paths.put(expression, path); return true; } diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/DeltaTableFilterPushdownProcessor.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/DeltaTableFilterPushdownProcessor.java index 8e5bf5be94..38dfde8132 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/DeltaTableFilterPushdownProcessor.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/DeltaTableFilterPushdownProcessor.java @@ -50,9 +50,9 @@ public class DeltaTableFilterPushdownProcessor extends ColumnFilterPushdownProce } @Override - protected boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException { - IExpectedSchemaNode node = expression.accept(exprToNodeVisitor, null); - if (node == null || node.getType() != ExpectedSchemaNodeType.ANY) { + protected boolean handlePath(AbstractFunctionCallExpression expression, IExpectedSchemaNode node) + throws AlgebricksException { + if (node.getType() != ExpectedSchemaNodeType.ANY) { return false; } diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ExternalDatasetFilterPushdownProcessor.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ExternalDatasetFilterPushdownProcessor.java index 1007313a9b..37a0128acb 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ExternalDatasetFilterPushdownProcessor.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ExternalDatasetFilterPushdownProcessor.java @@ -73,9 +73,9 @@ public class ExternalDatasetFilterPushdownProcessor extends ColumnFilterPushdown } @Override - protected boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException { - IExpectedSchemaNode node = expression.accept(exprToNodeVisitor, null); - if (node == null || node.getType() != ExpectedSchemaNodeType.ANY) { + protected boolean handlePath(AbstractFunctionCallExpression expression, IExpectedSchemaNode node) + throws AlgebricksException { + if (node.getType() != ExpectedSchemaNodeType.ANY) { return false; } diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ParquetFilterPushdownProcessor.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ParquetFilterPushdownProcessor.java index 25db9f9342..6545964a45 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ParquetFilterPushdownProcessor.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ParquetFilterPushdownProcessor.java @@ -32,7 +32,6 @@ import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; import org.apache.hyracks.algebricks.core.algebra.base.IOptimizationContext; import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression; -import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; public class ParquetFilterPushdownProcessor extends ColumnFilterPushdownProcessor { @@ -47,14 +46,13 @@ public class ParquetFilterPushdownProcessor extends ColumnFilterPushdownProcesso @Override protected boolean isNotPushable(AbstractFunctionCallExpression expression) { - FunctionIdentifier fid = expression.getFunctionIdentifier(); return !RANGE_FILTER_PUSHABLE_FUNCTIONS.contains(expression.getFunctionIdentifier()); } @Override - protected boolean handlePath(AbstractFunctionCallExpression expression) throws AlgebricksException { - IExpectedSchemaNode node = expression.accept(exprToNodeVisitor, null); - if (node == null || node.getType() != ExpectedSchemaNodeType.ANY) { + protected boolean handlePath(AbstractFunctionCallExpression expression, IExpectedSchemaNode node) + throws AlgebricksException { + if (node.getType() != ExpectedSchemaNodeType.ANY) { return false; } diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/column-filter/ASTERIXDB-3595.001.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/column-filter/ASTERIXDB-3595.001.sqlpp new file mode 100644 index 0000000000..9a516be6db --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/column-filter/ASTERIXDB-3595.001.sqlpp @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +DROP DATAVERSE test IF EXISTS; +CREATE DATAVERSE test; +USE test; + +CREATE DATASET Res1 PRIMARY KEY (id:int) WITH { + "storage-format": {"format" : "column"} +}; +CREATE DATASET Res2 PRIMARY KEY (id:int) WITH { + "storage-format": {"format" : "column"} +}; + +SET `compiler.column.filter` "true"; +SELECT value r1.id FROM Res1 r1, Res2 r2 +WHERE r1.id = r2.id AND (r1.sensitive OR r2.sensitive) +ORDER BY r1.id; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/column-filter/ASTERIXDB-3595.001.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/column-filter/ASTERIXDB-3595.001.plan new file mode 100644 index 0000000000..b373372bc9 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/column-filter/ASTERIXDB-3595.001.plan @@ -0,0 +1,38 @@ +distribute result [$$36] +-- DISTRIBUTE_RESULT |PARTITIONED| + exchange + -- SORT_MERGE_EXCHANGE [$$36(ASC) ] |PARTITIONED| + order (ASC, $$36) + -- STABLE_SORT [$$36(ASC)] |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + project ([$$36]) + -- STREAM_PROJECT |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + join (and(eq($$36, $$37), or($$39, $$40))) + -- HYBRID_HASH_JOIN [$$36][$$37] |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + assign [$$39] <- [$$r1.getField("sensitive")] project: [$$36, $$39] + -- ASSIGN |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + data-scan []<-[$$36, $$r1] <- test.Res1 project ({sensitive:any}) + -- DATASOURCE_SCAN |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + empty-tuple-source + -- EMPTY_TUPLE_SOURCE |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + assign [$$40] <- [$$r2.getField("sensitive")] project: [$$37, $$40] + -- ASSIGN |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + data-scan []<-[$$37, $$r2] <- test.Res2 project ({sensitive:any}) + -- DATASOURCE_SCAN |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + empty-tuple-source + -- EMPTY_TUPLE_SOURCE |PARTITIONED| \ No newline at end of file
