>From Ali Alsuliman <[email protected]>: Ali Alsuliman has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19635 )
Change subject: [ASTERIXDB-3595][COMP] Ensure path node exists when handling path ...................................................................... [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]> --- M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/AbstractFilterPushdownProcessor.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/DeltaTableFilterPushdownProcessor.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ParquetFilterPushdownProcessor.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnFilterPushdownProcessor.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ColumnRangeFilterPushdownProcessor.java A asterixdb/asterix-app/src/test/resources/optimizerts/results/column-filter/ASTERIXDB-3595.001.plan A asterixdb/asterix-app/src/test/resources/optimizerts/queries/column-filter/ASTERIXDB-3595.001.sqlpp M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/processor/ExternalDatasetFilterPushdownProcessor.java 8 files changed, 140 insertions(+), 19 deletions(-) Approvals: Ritik Raj: Looks good to me, approved Jenkins: Verified; Verified Anon. E. Moose #1000171: 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 7d4f7a5..92436c7 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.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 @@ * @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 0d79767..497e751 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 @@ } @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 @@ 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 75a7608..030fb6e 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 @@ } @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 8e5bf5b..38dfde8 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 @@ } @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 1007313..37a0128 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 @@ } @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 25db9f9..6545964 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.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 @@ @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 0000000..9a516be --- /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 0000000..b373372 --- /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 -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19635 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: ionic Gerrit-Change-Id: Ie11d88c6ace3dcc08744d3b04add679a6206a41c Gerrit-Change-Number: 19635 Gerrit-PatchSet: 6 Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-Reviewer: Ritik Raj <[email protected]> Gerrit-MessageType: merged
