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 cac1e4ab32b24ae35c657c72d9979f41029c45d7 Author: Wail Alkowaileet <wail.alkowail...@couchbase.com> AuthorDate: Fri Dec 9 12:03:40 2022 -0800 [ASTERIXDB-3092][COMP] Consider only field-access functions in LoadRecordFieldsRule - user model changes: no - storage format changes: no - interface changes: no Details: The compiler rule LoadRecordFieldsRule fails when it encounters a non-field-access function. This patch fixes this issue by ensuring that only field accesses are considered for this rule. Change-Id: I88f72fd51716dd8152e709c841489e87af0a5137 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17298 Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Wail Alkowaileet <wael....@gmail.com> Reviewed-by: Ali Alsuliman <ali.al.solai...@gmail.com> --- .../optimizer/rules/LoadRecordFieldsRule.java | 14 +++++----- .../queries_sqlpp/objects/ObjectsQueries.xml | 5 ++++ .../load-record-fields.1.ddl.sqlpp | 30 ++++++++++++++++++++++ .../load-record-fields.2.update.sqlpp | 25 ++++++++++++++++++ .../load-record-fields.3.query.sqlpp | 30 ++++++++++++++++++++++ .../load-record-fields.4.query.sqlpp | 30 ++++++++++++++++++++++ .../load-record-fields/load-record-fields.3.adm | 2 ++ .../load-record-fields/load-record-fields.4.plan | 26 +++++++++++++++++++ .../asterix/lang/common/util/FunctionUtil.java | 13 ++++++++++ 9 files changed, 168 insertions(+), 7 deletions(-) diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java index b9d512bb8e..42cce5233e 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java @@ -27,6 +27,7 @@ import java.util.List; import org.apache.asterix.algebra.base.OperatorAnnotation; import org.apache.asterix.common.exceptions.CompilationException; import org.apache.asterix.common.exceptions.ErrorCode; +import org.apache.asterix.lang.common.util.FunctionUtil; import org.apache.asterix.om.base.AInt32; import org.apache.asterix.om.base.AString; import org.apache.asterix.om.constants.AsterixConstantValue; @@ -64,7 +65,7 @@ import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule; public class LoadRecordFieldsRule implements IAlgebraicRewriteRule { - private ExtractFieldLoadExpressionVisitor exprVisitor = new ExtractFieldLoadExpressionVisitor(); + private final ExtractFieldLoadExpressionVisitor exprVisitor = new ExtractFieldLoadExpressionVisitor(); @Override public boolean rewritePre(Mutable<ILogicalOperator> opRef, IOptimizationContext context) { @@ -98,13 +99,13 @@ public class LoadRecordFieldsRule implements IAlgebraicRewriteRule { // checking if we can annotate a Selection as using just one field // access SelectOperator sigma = (SelectOperator) op1; - LinkedList<LogicalVariable> vars = new LinkedList<LogicalVariable>(); + List<LogicalVariable> vars = new ArrayList<>(); VariableUtilities.getUsedVariables(sigma, vars); if (vars.size() == 1) { // we can annotate Selection AssignOperator assign1 = (AssignOperator) op1.getInputs().get(0).getValue(); - AbstractLogicalExpression expr1 = (AbstractLogicalExpression) getFirstExpr(assign1); - if (expr1.getExpressionTag() == LogicalExpressionTag.FUNCTION_CALL) { + ILogicalExpression expr1 = getFirstExpr(assign1); + if (FunctionUtil.isFieldAccessFunction(expr1)) { AbstractFunctionCallExpression f = (AbstractFunctionCallExpression) expr1; // f should be a call to a field/data access kind of // function @@ -141,7 +142,7 @@ public class LoadRecordFieldsRule implements IAlgebraicRewriteRule { } // create an assign LogicalVariable v = context.newVar(); - AssignOperator a2 = new AssignOperator(v, new MutableObject<ILogicalExpression>(f)); + AssignOperator a2 = new AssignOperator(v, new MutableObject<>(f)); a2.setSourceLocation(expr.getSourceLocation()); pushFieldAssign(a2, topOp, context); context.computeAndSetTypeEnvironmentForOperator(a2); @@ -151,7 +152,7 @@ public class LoadRecordFieldsRule implements IAlgebraicRewriteRule { LogicalVariable var = ref.getVariableReference(); List<LogicalVariable> keys = context.findPrimaryKey(var); if (keys != null) { - List<LogicalVariable> tail = new ArrayList<LogicalVariable>(); + List<LogicalVariable> tail = new ArrayList<>(); tail.add(v); FunctionalDependency pk = new FunctionalDependency(keys, tail); context.addPrimaryKey(pk); @@ -408,5 +409,4 @@ public class LoadRecordFieldsRule implements IAlgebraicRewriteRule { private static ILogicalExpression getFirstExpr(AssignOperator assign) { return assign.getExpressions().get(0).getValue(); } - } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/ObjectsQueries.xml b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/ObjectsQueries.xml index 623237b550..8532fe9034 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/ObjectsQueries.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/ObjectsQueries.xml @@ -244,4 +244,9 @@ <expected-warn>Duplicate field name 'fname1' (in line 25, at column 45)</expected-warn> </compilation-unit> </test-case> + <test-case FilePath="objects"> + <compilation-unit name="load-record-fields"> + <output-dir compare="Text">load-record-fields</output-dir> + </compilation-unit> + </test-case> </test-group> diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.1.ddl.sqlpp new file mode 100644 index 0000000000..99898f9b28 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.1.ddl.sqlpp @@ -0,0 +1,30 @@ +/* + * 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 TYPE OpenType AS { + id: int +}; + +CREATE DATASET MyDataset(OpenType) +PRIMARY KEY id; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.2.update.sqlpp new file mode 100644 index 0000000000..1f80ae12c8 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.2.update.sqlpp @@ -0,0 +1,25 @@ +/* + * 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. + */ + +USE test; + +INSERT INTO MyDataset ( + {"id": 1, "name": "Alice"}, + {"id": 2, "name": "Bob"} +); \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.3.query.sqlpp new file mode 100644 index 0000000000..278f5d2b1e --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.3.query.sqlpp @@ -0,0 +1,30 @@ +/* + * 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. + */ + +USE test; + +-- Disabled for a simpler plan +SET `compiler.sort.parallel` "false"; + + +SELECT VALUE md.name +FROM MyDataset md +LET currentData = {"myDate": current_date()} +WHERE currentData.myDate = current_date() +ORDER BY md.id diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.4.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.4.query.sqlpp new file mode 100644 index 0000000000..f44921f99b --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/objects/load-record-fields/load-record-fields.4.query.sqlpp @@ -0,0 +1,30 @@ +/* + * 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. + */ + +USE test; + +-- Disabled for a simpler plan +SET `compiler.sort.parallel` "false"; + +EXPLAIN +SELECT VALUE md.name +FROM MyDataset md +LET currentData = {"myDate": current_date()} +WHERE currentData.myDate = current_date() +ORDER BY md.id diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/objects/load-record-fields/load-record-fields.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/objects/load-record-fields/load-record-fields.3.adm new file mode 100644 index 0000000000..ac2dc970eb --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/objects/load-record-fields/load-record-fields.3.adm @@ -0,0 +1,2 @@ +"Alice" +"Bob" diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/objects/load-record-fields/load-record-fields.4.plan b/asterixdb/asterix-app/src/test/resources/runtimets/results/objects/load-record-fields/load-record-fields.4.plan new file mode 100644 index 0000000000..5ac69f34ba --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/objects/load-record-fields/load-record-fields.4.plan @@ -0,0 +1,26 @@ +distribute result [$$28] +-- DISTRIBUTE_RESULT |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + project ([$$28]) + -- STREAM_PROJECT |PARTITIONED| + exchange + -- SORT_MERGE_EXCHANGE [$$30(ASC) ] |PARTITIONED| + project ([$$28, $$30]) + -- STREAM_PROJECT |PARTITIONED| + select (eq($$31, current-date())) + -- STREAM_SELECT |PARTITIONED| + assign [$$31] <- [current-date()] + -- ASSIGN |PARTITIONED| + project ([$$30, $$28]) + -- STREAM_PROJECT |PARTITIONED| + assign [$$28] <- [$$md.getField("name")] + -- ASSIGN |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + data-scan []<-[$$30, $$md] <- test.MyDataset + -- DATASOURCE_SCAN |PARTITIONED| + exchange + -- ONE_TO_ONE_EXCHANGE |PARTITIONED| + empty-tuple-source + -- EMPTY_TUPLE_SOURCE |PARTITIONED| diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java index 42a35f3406..1cf88c2c35 100644 --- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java +++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java @@ -58,6 +58,7 @@ import org.apache.commons.lang3.mutable.Mutable; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.common.utils.Triple; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; +import org.apache.hyracks.algebricks.core.algebra.base.LogicalExpressionTag; import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.api.exceptions.IWarningCollector; @@ -303,4 +304,16 @@ public class FunctionUtil { function.getSignature(), e.getMessage()); } } + + public static boolean isFieldAccessFunction(ILogicalExpression expression) { + if (expression.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL) { + return false; + } + + AbstractFunctionCallExpression funcExpr = (AbstractFunctionCallExpression) expression; + FunctionIdentifier fid = funcExpr.getFunctionIdentifier(); + + return BuiltinFunctions.FIELD_ACCESS_BY_INDEX.equals(fid) || BuiltinFunctions.FIELD_ACCESS_BY_NAME.equals(fid) + || BuiltinFunctions.FIELD_ACCESS_NESTED.equals(fid); + } }