Ali Alsuliman has submitted this change and it was merged. ( https://asterix-gerrit.ics.uci.edu/3406 )
Change subject: [ASTERIXDB-2458][COMP] Fix InjectTypeCastForFunctionArgumentsRule ...................................................................... [ASTERIXDB-2458][COMP] Fix InjectTypeCastForFunctionArgumentsRule - user model changes: no - storage format changes: no - interface changes: no Details: InjectTypeCastForFunctionArgumentsRule is for functions that can potentially return any of their arguments. switch and if_null(expr1, expr2, ...) are examples. All the arguments need to be casted (opened) to the type that the function will return which is the generalized type of all arguments. Some functions like if_null can determine the exact expression they will return, e.g. if_null(1, {"id": 3}) in which case the return type is always integer. The rule tries to cast th 2nd argument, the record, to integer and fails. In such cases, these functions do not need to cast their arguments. If the function determines its output type to be ANY, then all arguments need to be casted (opened). If the function determines its output to be a dervied type, then casting is also needed since that output type should be the generalized type of all arguments. Change-Id: I2fee234d883b59319e4ec4df58d61ecd498373fd Reviewed-on: https://asterix-gerrit.ics.uci.edu/3406 Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Dmitry Lychagin <dmitry.lycha...@couchbase.com> --- M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/results/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.adm M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml M asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/TypeResolverUtil.java 5 files changed, 64 insertions(+), 22 deletions(-) Approvals: Jenkins: Verified; No violations found; ; Verified Anon. E. Moose (1000171): Dmitry Lychagin: Looks good to me, approved diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java index 3678826..77d63f1 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java @@ -24,12 +24,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.IntBinaryOperator; import java.util.function.IntPredicate; import org.apache.asterix.dataflow.data.common.TypeResolverUtil; import org.apache.asterix.lang.common.util.FunctionUtil; import org.apache.asterix.om.functions.BuiltinFunctions; import org.apache.asterix.om.typecomputer.base.TypeCastUtils; +import org.apache.asterix.om.typecomputer.impl.TypeComputeUtils; +import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.IAType; import org.apache.commons.lang3.mutable.Mutable; import org.apache.commons.lang3.mutable.MutableObject; @@ -103,39 +106,27 @@ } FunctionIdentifier funcId = func.getFunctionIdentifier(); if (funcId.equals(BuiltinFunctions.SWITCH_CASE)) { - rewritten |= rewriteSwitchCase(op, func, context); + rewritten |= rewriteFunction(op, func, null, context, 2, + InjectTypeCastForFunctionArgumentsRule::switchIncrement); } else if (FUN_TO_ARG_CHECKER.containsKey(funcId)) { - rewritten |= rewriteFunction(op, func, FUN_TO_ARG_CHECKER.get(funcId), context); - } - return rewritten; - } - - // Injects casts that cast types for different "THEN" and "ELSE" branches. - private boolean rewriteSwitchCase(ILogicalOperator op, AbstractFunctionCallExpression func, - IOptimizationContext context) throws AlgebricksException { - IVariableTypeEnvironment env = op.computeInputTypeEnvironment(context); - IAType producedType = (IAType) env.getType(func); - List<Mutable<ILogicalExpression>> argRefs = func.getArguments(); - int argSize = argRefs.size(); - boolean rewritten = false; - for (int argIndex = 2; argIndex < argSize; argIndex += (argIndex + 2 == argSize) ? 1 : 2) { - Mutable<ILogicalExpression> argRef = argRefs.get(argIndex); - if (rewriteFunctionArgument(argRef, producedType, env)) { - rewritten = true; - } + rewritten |= rewriteFunction(op, func, FUN_TO_ARG_CHECKER.get(funcId), context, 0, + InjectTypeCastForFunctionArgumentsRule::increment); } return rewritten; } // Injects casts that cast types for all function parameters private boolean rewriteFunction(ILogicalOperator op, AbstractFunctionCallExpression func, IntPredicate argChecker, - IOptimizationContext context) throws AlgebricksException { + IOptimizationContext context, int argStartIdx, IntBinaryOperator increment) throws AlgebricksException { IVariableTypeEnvironment env = op.computeInputTypeEnvironment(context); IAType producedType = (IAType) env.getType(func); + if (!argumentsNeedCasting(producedType)) { + return false; + } List<Mutable<ILogicalExpression>> argRefs = func.getArguments(); int argSize = argRefs.size(); boolean rewritten = false; - for (int argIndex = 0; argIndex < argSize; argIndex++) { + for (int argIndex = argStartIdx; argIndex < argSize; argIndex += increment.applyAsInt(argIndex, argSize)) { if (argChecker == null || argChecker.test(argIndex)) { rewritten |= rewriteFunctionArgument(argRefs.get(argIndex), producedType, env); } @@ -159,4 +150,18 @@ } return false; } + + private static boolean argumentsNeedCasting(IAType functionProducedType) { + ATypeTag functionProducedTag = TypeComputeUtils.getActualType(functionProducedType).getTypeTag(); + return functionProducedTag == ATypeTag.ANY || functionProducedTag.isDerivedType(); + } + + private static int switchIncrement(int currentArgIndex, int numArguments) { + return currentArgIndex + 2 == numArguments ? 1 : 2; + } + + @SuppressWarnings("squid:S1172") // unused parameter + private static int increment(int currentArgIndex, int numArguments) { + return 1; + } } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.query.sqlpp new file mode 100644 index 0000000..cb7f81e --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.query.sqlpp @@ -0,0 +1,26 @@ +/* + * 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. + */ + +FROM [ +if_missing(1, {"id": 4}), +if_missing({"id": 4}, 1), +if_missing_or_null(1, {"id": 4}), +if_missing_or_null({"id": 4}, 1), +if_null(1, {"id": 4}), +if_null({"id": 4}, 1)] as v select v; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.adm new file mode 100644 index 0000000..07e8234 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/cast-ASTERIXDB-2458/cast-ASTERIXDB-2458.1.adm @@ -0,0 +1,6 @@ +{ "v": 1 } +{ "v": { "id": 4 } } +{ "v": 1 } +{ "v": { "id": 4 } } +{ "v": 1 } +{ "v": { "id": 4 } } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 1d21dfa..fae2795 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -6032,6 +6032,11 @@ <output-dir compare="Text">metadata_only_01</output-dir> </compilation-unit> </test-case> + <test-case FilePath="misc"> + <compilation-unit name="cast-ASTERIXDB-2458"> + <output-dir compare="Text">cast-ASTERIXDB-2458</output-dir> + </compilation-unit> + </test-case> </test-group> <test-group name="index"> <test-group name="index/validations"> diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/TypeResolverUtil.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/TypeResolverUtil.java index b7bd8ca..3cc9e3d 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/TypeResolverUtil.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/TypeResolverUtil.java @@ -89,7 +89,7 @@ } // Casts are only needed when the original return type is a complex type. // (In the runtime, there is already a type tag for scalar types.) - if (tag != ATypeTag.OBJECT && tag != ATypeTag.MULTISET && tag != ATypeTag.ARRAY) { + if (!tag.isDerivedType()) { return false; } return !TypeComputeUtils.getActualType(reqType).equals(TypeComputeUtils.getActualType(inputType)); -- To view, visit https://asterix-gerrit.ics.uci.edu/3406 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2fee234d883b59319e4ec4df58d61ecd498373fd Gerrit-Change-Number: 3406 Gerrit-PatchSet: 4 Gerrit-Owner: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose (1000171) Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org>