[NO ISSUE][COMP] allow optimization rule to inspect other functions + fixes
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
1. allow the optimization rule "InjectTypeCastForSwitchCaseRule"
(renamed to "InjectTypeCastForFunctionArgumentsRule" now) to consider other
functions specified by the user in addition to the ones already considered
by the rule.
2. fix an issue related to ConstantFoldingRule that affects some existing
functions where the rule would have the function compute the value but fail
to deserialize it (read it) back due to differences in compile-time type
and runtime type.
3. fix static casting of a list constructor function which fails to cast
open its items when the item is a function that returns a closed derived
type.
Change-Id: I65679e934a41e00b04bc3fd479ed404af293eb21
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2980
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java
index f9b5c38..cf01573 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java
@@ -41,7 +41,7 @@
import org.apache.asterix.optimizer.rules.FixReplicateOperatorOutputsRule;
import org.apache.asterix.optimizer.rules.FullTextContainsParameterCheckRule;
import org.apache.asterix.optimizer.rules.FuzzyEqRule;
-import org.apache.asterix.optimizer.rules.InjectTypeCastForSwitchCaseRule;
+import org.apache.asterix.optimizer.rules.InjectTypeCastForFunctionArgumentsRule;
import org.apache.asterix.optimizer.rules.InjectTypeCastForUnionRule;
import org.apache.asterix.optimizer.rules.InlineUnnestFunctionRule;
import org.apache.asterix.optimizer.rules.IntroduceAutogenerateIDRule;
@@ -306,7 +306,7 @@
planCleanupRules.add(new IntroduceDynamicTypeCastForExternalFunctionRule());
planCleanupRules.add(new RemoveUnusedAssignAndAggregateRule());
planCleanupRules.add(new RemoveCartesianProductWithEmptyBranchRule());
- planCleanupRules.add(new InjectTypeCastForSwitchCaseRule());
+ planCleanupRules.add(new InjectTypeCastForFunctionArgumentsRule());
planCleanupRules.add(new InjectTypeCastForUnionRule());
// Needs to invoke ByNameToByIndexFieldAccessRule as the last logical optimization rule because
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
index a2c500a..f10ae1a 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
@@ -43,10 +43,14 @@
import org.apache.asterix.om.constants.AsterixConstantValue;
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.ARecordType;
import org.apache.asterix.om.types.ATypeTag;
import org.apache.asterix.om.types.AbstractCollectionType;
+import org.apache.asterix.om.types.IAType;
+import org.apache.asterix.om.types.TypeTagUtil;
import org.apache.asterix.om.utils.ConstantExpressionUtil;
+import org.apache.asterix.runtime.evaluators.functions.PointableHelper;
import org.apache.commons.lang3.mutable.Mutable;
import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException;
import org.apache.hyracks.algebricks.common.utils.Pair;
@@ -237,11 +241,16 @@
IScalarEvaluator eval = fact.createScalarEvaluator(null);
eval.evaluate(null, p);
- Object t = _emptyTypeEnv.getType(expr);
-
+ IAType returnType = (IAType) _emptyTypeEnv.getType(expr);
+ ATypeTag runtimeType = PointableHelper.getTypeTag(p);
+ if (runtimeType.isDerivedType()) {
+ returnType = TypeComputeUtils.getActualType(returnType);
+ } else {
+ returnType = TypeTagUtil.getBuiltinTypeByTag(runtimeType);
+ }
@SuppressWarnings("rawtypes")
ISerializerDeserializer serde =
- jobGenCtx.getSerializerDeserializerProvider().getSerializerDeserializer(t);
+ jobGenCtx.getSerializerDeserializerProvider().getSerializerDeserializer(returnType);
bbis.setByteBuffer(ByteBuffer.wrap(p.getByteArray(), p.getStartOffset(), p.getLength()), 0);
IAObject o = (IAObject) serde.deserialize(dis);
return new Pair<>(true, new ConstantExpression(new AsterixConstantValue(o)));
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForSwitchCaseRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java
similarity index 84%
rename from asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForSwitchCaseRule.java
rename to asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java
index 683d29f..d0107c0 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForSwitchCaseRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InjectTypeCastForFunctionArgumentsRule.java
@@ -21,8 +21,10 @@
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
-import java.util.Set;
+import java.util.Map;
+import java.util.function.IntPredicate;
import org.apache.asterix.dataflow.data.common.TypeResolverUtil;
import org.apache.asterix.lang.common.util.FunctionUtil;
@@ -42,8 +44,6 @@
import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier;
import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
-import com.google.common.collect.ImmutableSet;
-
/**
* This rule injects casts for function parameters if they have heterogeneous return types:
* <ul>
@@ -51,10 +51,20 @@
* <li>for parameters of "if missing/null" functions (if-missing(), if-null(), if-missing-or-null())</li>
* </ul>
*/
-public class InjectTypeCastForSwitchCaseRule implements IAlgebraicRewriteRule {
+public class InjectTypeCastForFunctionArgumentsRule implements IAlgebraicRewriteRule {
- private static final Set<FunctionIdentifier> IF_FUNCTIONS =
- ImmutableSet.of(BuiltinFunctions.IF_MISSING, BuiltinFunctions.IF_NULL, BuiltinFunctions.IF_MISSING_OR_NULL);
+ private static final Map<FunctionIdentifier, IntPredicate> FUN_TO_ARG_CHECKER = new HashMap<>();
+
+ static {
+ addFunctionAndArgChecker(BuiltinFunctions.IF_MISSING, null);
+ addFunctionAndArgChecker(BuiltinFunctions.IF_NULL, null);
+ addFunctionAndArgChecker(BuiltinFunctions.IF_MISSING_OR_NULL, null);
+ }
+
+ // allows the rule to check other functions in addition to the ones specified here
+ public static void addFunctionAndArgChecker(FunctionIdentifier function, IntPredicate argChecker) {
+ FUN_TO_ARG_CHECKER.put(function, argChecker);
+ }
@Override
public boolean rewritePost(Mutable<ILogicalOperator> opRef, IOptimizationContext context)
@@ -92,13 +102,9 @@
}
FunctionIdentifier funcId = func.getFunctionIdentifier();
if (funcId.equals(BuiltinFunctions.SWITCH_CASE)) {
- if (rewriteSwitchCase(op, func, context)) {
- rewritten = true;
- }
- } else if (IF_FUNCTIONS.contains(funcId)) {
- if (rewriteFunction(op, func, context)) {
- rewritten = true;
- }
+ rewritten |= rewriteSwitchCase(op, func, context);
+ } else if (FUN_TO_ARG_CHECKER.containsKey(funcId)) {
+ rewritten |= rewriteFunction(op, func, FUN_TO_ARG_CHECKER.get(funcId), context);
}
return rewritten;
}
@@ -121,7 +127,7 @@
}
// Injects casts that cast types for all function parameters
- private boolean rewriteFunction(ILogicalOperator op, AbstractFunctionCallExpression func,
+ private boolean rewriteFunction(ILogicalOperator op, AbstractFunctionCallExpression func, IntPredicate argChecker,
IOptimizationContext context) throws AlgebricksException {
IVariableTypeEnvironment env = op.computeInputTypeEnvironment(context);
IAType producedType = (IAType) env.getType(func);
@@ -129,9 +135,8 @@
int argSize = argRefs.size();
boolean rewritten = false;
for (int argIndex = 0; argIndex < argSize; argIndex++) {
- Mutable<ILogicalExpression> argRef = argRefs.get(argIndex);
- if (rewriteFunctionArgument(argRef, producedType, env)) {
- rewritten = true;
+ if (argChecker == null || argChecker.test(argIndex)) {
+ rewritten |= rewriteFunctionArgument(argRefs.get(argIndex), producedType, env);
}
}
return rewritten;
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/typecast/StaticTypeCastUtil.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/typecast/StaticTypeCastUtil.java
index 4b2ff76..55b174b 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/typecast/StaticTypeCastUtil.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/typecast/StaticTypeCastUtil.java
@@ -27,6 +27,7 @@
import org.apache.asterix.common.exceptions.CompilationException;
import org.apache.asterix.common.exceptions.ErrorCode;
+import org.apache.asterix.dataflow.data.common.TypeResolverUtil;
import org.apache.asterix.lang.common.util.FunctionUtil;
import org.apache.asterix.om.base.ANull;
import org.apache.asterix.om.base.AString;
@@ -73,9 +74,9 @@
*
* @param funcExpr
* record constructor function expression
- * @param requiredListType
+ * @param reqType
* required record type
- * @param inputRecordType
+ * @param inputType
* @param env
* type environment
* @throws AlgebricksException
@@ -179,7 +180,7 @@
*
* @param funcExpr
* record constructor function expression
- * @param requiredListType
+ * @param requiredRecordType
* required record type
* @param inputRecordType
* @param env
@@ -223,7 +224,7 @@
TypeCastUtils.setRequiredAndInputTypes(funcExpr, requiredListType, inputListType);
List<Mutable<ILogicalExpression>> args = funcExpr.getArguments();
- IAType itemType = requiredListType.getItemType();
+ IAType requiredItemType = requiredListType.getItemType();
IAType inputItemType = inputListType.getItemType();
boolean changed = false;
for (int j = 0; j < args.size(); j++) {
@@ -233,7 +234,8 @@
switch (arg.getExpressionTag()) {
case FUNCTION_CALL:
ScalarFunctionCallExpression argFunc = (ScalarFunctionCallExpression) arg;
- changed = rewriteFuncExpr(argFunc, itemType, currentItemType, env) || changed;
+ changed = rewriteFuncExpr(argFunc, requiredItemType, currentItemType, env) || changed;
+ changed |= castItem(requiredItemType, currentItemType, argFunc, args.get(j));
break;
case VARIABLE:
changed = injectCastToRelaxType(args.get(j), currentItemType, env) || changed;
@@ -243,6 +245,20 @@
return changed;
}
+ private static boolean castItem(IAType requiredItemType, IAType currentItemType,
+ ScalarFunctionCallExpression itemExpr, Mutable<ILogicalExpression> itemExprRef) throws AlgebricksException {
+ if (TypeResolverUtil.needsCast(requiredItemType, currentItemType) && shouldCast(itemExpr)) {
+ injectCastFunction(FunctionUtil.getFunctionInfo(BuiltinFunctions.CAST_TYPE), requiredItemType,
+ currentItemType, itemExprRef, itemExpr);
+ return true;
+ }
+ return false;
+ }
+
+ private static boolean shouldCast(ScalarFunctionCallExpression itemExpr) {
+ return TypeCastUtils.getRequiredType(itemExpr) == null;
+ }
+
/**
* This method statically cast the type of records from their current type to the required type.
*
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/list/enforcing_item_type/enforcing_item_type.0.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/list/enforcing_item_type/enforcing_item_type.0.query.sqlpp
new file mode 100644
index 0000000..522a352
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/list/enforcing_item_type/enforcing_item_type.0.query.sqlpp
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: testing that each item conforms to the list type including function items.
+ */
+
+select value {"id": t[0], "v": t[1]}
+from [
+[ 29, CASE(2 < 1) WHEN true THEN null ELSE {"f1": "a", "f2": 3} END]
+] t;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/list/enforcing_item_type/enforcing_item_type.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/list/enforcing_item_type/enforcing_item_type.1.query.sqlpp
new file mode 100644
index 0000000..0abb216
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/list/enforcing_item_type/enforcing_item_type.1.query.sqlpp
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: testing that each item conforms to the list type including function items.
+ */
+
+explain select value {"id": t[0], "v": t[1]}
+from [
+[ 29, CASE(2 < 1) WHEN true THEN null ELSE {"f1": "a", "f2": 3} END]
+] t;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.0.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.0.query.sqlpp
new file mode 100644
index 0000000..671ff7d
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.0.query.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.
+ */
+
+/*
+ * Description: testing constant folding of functions that return missable/nullable closed-type object
+ */
+
+select value CASE(2 < 1) WHEN true THEN null ELSE { "f1": 5, "f2": 6, "f3": 7 } END;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.1.query.sqlpp
new file mode 100644
index 0000000..ad5f59b
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.1.query.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.
+ */
+
+/*
+ * Description: testing constant folding of functions that return missable/nullable closed-type object
+ */
+
+explain select value CASE(2 < 1) WHEN true THEN null ELSE { "f1": 5, "f2": 6, "f3": 7 } END;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/list/enforcing_item_type/enforcing_item_type.0.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/list/enforcing_item_type/enforcing_item_type.0.adm
new file mode 100644
index 0000000..325fe87
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/list/enforcing_item_type/enforcing_item_type.0.adm
@@ -0,0 +1 @@
+{ "id": 29, "v": { "f1": "a", "f2": 3 } }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/list/enforcing_item_type/enforcing_item_type.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/list/enforcing_item_type/enforcing_item_type.1.adm
new file mode 100644
index 0000000..fbe0345
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/list/enforcing_item_type/enforcing_item_type.1.adm
@@ -0,0 +1,12 @@
+distribute result [$$18]
+-- DISTRIBUTE_RESULT |UNPARTITIONED|
+ exchange
+ -- ONE_TO_ONE_EXCHANGE |UNPARTITIONED|
+ project ([$$18])
+ -- STREAM_PROJECT |UNPARTITIONED|
+ assign [$$18] <- [{"id": get-item($$t, 0), "v": get-item($$t, 1)}]
+ -- ASSIGN |UNPARTITIONED|
+ unnest $$t <- scan-collection(ordered-list-constructor(ordered-list-constructor(29, cast({ f1: "a", f2: 3 }))))
+ -- UNNEST |UNPARTITIONED|
+ empty-tuple-source
+ -- EMPTY_TUPLE_SOURCE |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.0.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.0.adm
new file mode 100644
index 0000000..61265f7
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.0.adm
@@ -0,0 +1 @@
+{ "f1": 5, "f2": 6, "f3": 7 }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.1.adm
new file mode 100644
index 0000000..379603f
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.1.adm
@@ -0,0 +1,8 @@
+distribute result [$$5]
+-- DISTRIBUTE_RESULT |UNPARTITIONED|
+ exchange
+ -- ONE_TO_ONE_EXCHANGE |UNPARTITIONED|
+ assign [$$5] <- [{ f1: 5, f2: 6, f3: 7 }]
+ -- ASSIGN |UNPARTITIONED|
+ empty-tuple-source
+ -- EMPTY_TUPLE_SOURCE |UNPARTITIONED|
\ 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 398b58b..52cb6e5 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -3673,6 +3673,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="list">
+ <compilation-unit name="enforcing_item_type">
+ <output-dir compare="Text">enforcing_item_type</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="list">
<compilation-unit name="exists">
<output-dir compare="Text">exists</output-dir>
</compilation-unit>
@@ -3791,6 +3796,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="misc">
+ <compilation-unit name="constant_folding">
+ <output-dir compare="Text">constant_folding</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="misc">
<compilation-unit name="poll-dynamic">
<output-dir compare="Text">poll-dynamic</output-dir>
</compilation-unit>