ASTERIXDB-1901 Fix IntroduceDynamicTypeCastForExternalFunctionRule
1. Instead of pattern matching, now we will inspect every paramter of
UDF. If there is a type mismatch, a cast function will be added.
2. Fixed the issue that type casting only applies to the first argument.
3. Added test case for this.
Change-Id: I6f44b2460ae3322fc52451e7939b6b5e711790a7
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1730
Reviewed-by: Yingyi Bu <buyingyi@gmail.com>
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastForExternalFunctionRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastForExternalFunctionRule.java
index a813285..c9a873b 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastForExternalFunctionRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastForExternalFunctionRule.java
@@ -19,26 +19,25 @@
package org.apache.asterix.optimizer.rules;
-import java.util.ArrayList;
-import java.util.List;
-
+import org.apache.asterix.lang.common.util.FunctionUtil;
import org.apache.asterix.metadata.functions.ExternalScalarFunctionInfo;
import org.apache.asterix.om.functions.BuiltinFunctions;
+import org.apache.asterix.om.typecomputer.base.TypeCastUtils;
import org.apache.asterix.om.types.ARecordType;
import org.apache.asterix.om.types.AUnionType;
+import org.apache.asterix.om.types.BuiltinType;
import org.apache.asterix.om.types.IAType;
import org.apache.asterix.om.utils.NonTaggedFormatUtil;
import org.apache.commons.lang3.mutable.Mutable;
+import org.apache.commons.lang3.mutable.MutableObject;
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.ILogicalOperator;
import org.apache.hyracks.algebricks.core.algebra.base.IOptimizationContext;
import org.apache.hyracks.algebricks.core.algebra.base.LogicalExpressionTag;
import org.apache.hyracks.algebricks.core.algebra.base.LogicalOperatorTag;
-import org.apache.hyracks.algebricks.core.algebra.base.LogicalVariable;
-import org.apache.hyracks.algebricks.core.algebra.expressions.IVariableTypeEnvironment;
+import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression;
import org.apache.hyracks.algebricks.core.algebra.expressions.ScalarFunctionCallExpression;
-import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractLogicalOperator;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.AssignOperator;
import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
@@ -56,94 +55,69 @@
return false;
}
+ private boolean rewriteFunctionArgs(ILogicalOperator op, Mutable<ILogicalExpression> expRef,
+ IOptimizationContext context) throws AlgebricksException {
+ ILogicalExpression expr = expRef.getValue();
+ if (expr.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL
+ || !(expr instanceof ScalarFunctionCallExpression)) {
+ return false;
+ }
+ ScalarFunctionCallExpression funcCallExpr = (ScalarFunctionCallExpression) expr;
+ boolean changed = false;
+ IAType inputRecordType;
+ ARecordType requiredRecordType;
+ for (int iter1 = 0; iter1 < funcCallExpr.getArguments().size(); iter1++) {
+ inputRecordType = (IAType) op.computeOutputTypeEnvironment(context)
+ .getType(funcCallExpr.getArguments().get(iter1).getValue());
+ if (!(((ExternalScalarFunctionInfo) funcCallExpr.getFunctionInfo()).getArgumenTypes()
+ .get(iter1) instanceof ARecordType)) {
+ continue;
+ }
+ requiredRecordType = (ARecordType) ((ExternalScalarFunctionInfo) funcCallExpr.getFunctionInfo())
+ .getArgumenTypes().get(iter1);
+ /**
+ * the input record type can be an union type
+ * for the case when it comes from a subplan or left-outer join
+ */
+ boolean checkUnknown = false;
+ while (NonTaggedFormatUtil.isOptional(inputRecordType)) {
+ /** while-loop for the case there is a nested multi-level union */
+ inputRecordType = ((AUnionType) inputRecordType).getActualType();
+ checkUnknown = true;
+ }
+ boolean castFlag = !IntroduceDynamicTypeCastRule.compatible(requiredRecordType, inputRecordType);
+ if (castFlag || checkUnknown) {
+ AbstractFunctionCallExpression castFunc = new ScalarFunctionCallExpression(
+ FunctionUtil.getFunctionInfo(BuiltinFunctions.CAST_TYPE));
+ castFunc.getArguments().add(funcCallExpr.getArguments().get(iter1));
+ TypeCastUtils.setRequiredAndInputTypes(castFunc, requiredRecordType, inputRecordType);
+ funcCallExpr.getArguments().set(iter1, new MutableObject<>(castFunc));
+ changed = changed || true;
+ }
+ }
+ return changed;
+ }
+
@Override
public boolean rewritePost(Mutable<ILogicalOperator> opRef, IOptimizationContext context)
throws AlgebricksException {
- /**
- * pattern match: distribute_result - project - assign (external function call) - assign (open_record_constructor)
- * resulting plan: distribute_result - project - assign (external function call) - assign (cast-record) - assign(open_record_constructor)
- */
- AbstractLogicalOperator op1 = (AbstractLogicalOperator) opRef.getValue();
- if (op1.getOperatorTag() != LogicalOperatorTag.DISTRIBUTE_RESULT) {
+ AbstractLogicalOperator op = (AbstractLogicalOperator) opRef.getValue();
+ if (op.getOperatorTag() != LogicalOperatorTag.ASSIGN) {
return false;
}
- AbstractLogicalOperator op2 = (AbstractLogicalOperator) op1.getInputs().get(0).getValue();
- if (op2.getOperatorTag() != LogicalOperatorTag.PROJECT) {
+ AssignOperator assignOp = (AssignOperator) op;
+ ILogicalExpression assignExpr = assignOp.getExpressions().get(0).getValue();
+ if (assignExpr.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL) {
return false;
}
- AbstractLogicalOperator op3 = (AbstractLogicalOperator) op2.getInputs().get(0).getValue();
- if (op3.getOperatorTag() != LogicalOperatorTag.ASSIGN) {
+ if (BuiltinFunctions.getBuiltinFunctionIdentifier(
+ ((AbstractFunctionCallExpression) assignExpr).getFunctionIdentifier()) != null) {
return false;
}
- AbstractLogicalOperator op4 = (AbstractLogicalOperator) op3.getInputs().get(0).getValue();
- if (op4.getOperatorTag() != LogicalOperatorTag.ASSIGN) {
- return false;
- }
-
- // Op1 : assign (external function call), Op2 : assign (open_record_constructor)
- AssignOperator assignOp1 = (AssignOperator) op3;
- AssignOperator assignOp2 = (AssignOperator) op4;
-
- // Checks whether open-record-constructor is called to create a record in the first assign operator - assignOp2
- FunctionIdentifier fid = null;
- ILogicalExpression assignExpr = assignOp2.getExpressions().get(0).getValue();
- if (assignExpr.getExpressionTag() == LogicalExpressionTag.FUNCTION_CALL) {
- ScalarFunctionCallExpression funcExpr =
- (ScalarFunctionCallExpression) assignOp2.getExpressions().get(0).getValue();
- fid = funcExpr.getFunctionIdentifier();
-
- if (fid != BuiltinFunctions.OPEN_RECORD_CONSTRUCTOR) {
- return false;
- }
+ if (op.acceptExpressionTransform(exprRef -> rewriteFunctionArgs(op, exprRef, context))) {
+ return true;
} else {
return false;
}
-
- // Checks whether an external function is called in the second assign operator - assignOp1
- assignExpr = assignOp1.getExpressions().get(0).getValue();
- ScalarFunctionCallExpression funcExpr = null;
- if (assignExpr.getExpressionTag() == LogicalExpressionTag.FUNCTION_CALL) {
- funcExpr = (ScalarFunctionCallExpression) assignOp1.getExpressions().get(0).getValue();
- fid = funcExpr.getFunctionIdentifier();
-
- // Checks whether this is an internal function call. Then, we return false.
- if (BuiltinFunctions.getBuiltinFunctionIdentifier(fid) != null) {
- return false;
- }
-
- } else {
- return false;
- }
-
- ExternalScalarFunctionInfo finfo = (ExternalScalarFunctionInfo) funcExpr.getFunctionInfo();
- ARecordType requiredRecordType = (ARecordType) finfo.getArgumenTypes().get(0);
-
- List<LogicalVariable> recordVar = new ArrayList<LogicalVariable>();
- recordVar.addAll(assignOp2.getVariables());
-
- IVariableTypeEnvironment env = assignOp2.computeOutputTypeEnvironment(context);
- IAType inputRecordType = (IAType) env.getVarType(recordVar.get(0));
-
- /** the input record type can be an union type -- for the case when it comes from a subplan or left-outer join */
- boolean checkUnknown = false;
- while (NonTaggedFormatUtil.isOptional(inputRecordType)) {
- /** while-loop for the case there is a nested multi-level union */
- inputRecordType = ((AUnionType) inputRecordType).getActualType();
- checkUnknown = true;
- }
-
- /** see whether the input record type needs to be casted */
- boolean cast = !IntroduceDynamicTypeCastRule.compatible(requiredRecordType, inputRecordType);
-
- if (checkUnknown) {
- recordVar.set(0, IntroduceDynamicTypeCastRule.addWrapperFunction(requiredRecordType, recordVar.get(0),
- assignOp1, context, BuiltinFunctions.CHECK_UNKNOWN));
- }
- if (cast) {
- IntroduceDynamicTypeCastRule.addWrapperFunction(requiredRecordType, recordVar.get(0), assignOp1, context,
- BuiltinFunctions.CAST_TYPE);
- }
- return cast || checkUnknown;
}
-
}
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.1.ddl.aql b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.1.ddl.aql
new file mode 100644
index 0000000..2dcd24d
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.1.ddl.aql
@@ -0,0 +1,27 @@
+/*
+ * 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 externallibtest if exists;
+create dataverse externallibtest;
+use dataverse externallibtest;
+
+create type TextType if not exists as open {
+ id: int32,
+ text: string
+};
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.2.lib.aql b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.2.lib.aql
new file mode 100644
index 0000000..d1e0e87
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.2.lib.aql
@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+install externallibtest testlib target/data/externallib/asterix-external-data-testlib.zip
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.3.query.aql b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.3.query.aql
new file mode 100644
index 0000000..61ed394
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.3.query.aql
@@ -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 dataverse externallibtest;
+
+let $i:={"id":1, "text":"lower text"}
+return testlib#toUpper($i);
+
+let $i:=testlib#toUpper({"id":1, "text":"lower text"})
+return $i;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.4.lib.aql b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.4.lib.aql
new file mode 100644
index 0000000..86af80f
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.4.lib.aql
@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+uninstall externallibtest testlib
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/upperCase/upperCase.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/upperCase/upperCase.1.adm
new file mode 100644
index 0000000..f475435
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/upperCase/upperCase.1.adm
@@ -0,0 +1,2 @@
+{ "id": -1, "text": "LOWER TEXT" }
+{ "id": -1, "text": "LOWER TEXT" }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it.xml
index 6609070..ed0ae7a 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it.xml
@@ -42,6 +42,11 @@
<output-dir compare="Text">getCapital</output-dir>
</compilation-unit>
</test-case>
+ <test-case FilePath="external-library">
+ <compilation-unit name="upperCase">
+ <output-dir compare="Text">upperCase</output-dir>
+ </compilation-unit>
+ </test-case>
</test-group>
<test-group name="feeds">
<test-case FilePath="feeds">