Repository: asterixdb Updated Branches: refs/heads/master 735532e43 -> bacf0c595
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 <buyin...@gmail.com> Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/bacf0c59 Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/bacf0c59 Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/bacf0c59 Branch: refs/heads/master Commit: bacf0c595eb0172522ce88a2600a6deaaafbc98b Parents: 735532e Author: Xikui Wang <xkk...@gmail.com> Authored: Thu May 11 17:08:23 2017 -0700 Committer: Yingyi Bu <buyin...@gmail.com> Committed: Thu May 11 18:26:15 2017 -0700 ---------------------------------------------------------------------- ...eDynamicTypeCastForExternalFunctionRule.java | 140 ++++++++----------- .../upperCase/upperCase.1.ddl.aql | 27 ++++ .../upperCase/upperCase.2.lib.aql | 19 +++ .../upperCase/upperCase.3.query.aql | 25 ++++ .../upperCase/upperCase.4.lib.aql | 19 +++ .../external-library/upperCase/upperCase.1.adm | 2 + .../test/resources/runtimets/testsuite_it.xml | 5 + 7 files changed, 154 insertions(+), 83 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastForExternalFunctionRule.java ---------------------------------------------------------------------- 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 @@ public class IntroduceDynamicTypeCastForExternalFunctionRule implements IAlgebra return false; } - @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) { + 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; } - AbstractLogicalOperator op2 = (AbstractLogicalOperator) op1.getInputs().get(0).getValue(); - if (op2.getOperatorTag() != LogicalOperatorTag.PROJECT) { - 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; + } } - AbstractLogicalOperator op3 = (AbstractLogicalOperator) op2.getInputs().get(0).getValue(); - if (op3.getOperatorTag() != LogicalOperatorTag.ASSIGN) { + return changed; + } + + @Override + public boolean rewritePost(Mutable<ILogicalOperator> opRef, IOptimizationContext context) + throws AlgebricksException { + AbstractLogicalOperator op = (AbstractLogicalOperator) opRef.getValue(); + if (op.getOperatorTag() != LogicalOperatorTag.ASSIGN) { return false; } - AbstractLogicalOperator op4 = (AbstractLogicalOperator) op3.getInputs().get(0).getValue(); - if (op4.getOperatorTag() != LogicalOperatorTag.ASSIGN) { + AssignOperator assignOp = (AssignOperator) op; + ILogicalExpression assignExpr = assignOp.getExpressions().get(0).getValue(); + if (assignExpr.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL) { 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; - } - } else { + if (BuiltinFunctions.getBuiltinFunctionIdentifier( + ((AbstractFunctionCallExpression) assignExpr).getFunctionIdentifier()) != null) { 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; - } - + if (op.acceptExpressionTransform(exprRef -> rewriteFunctionArgs(op, exprRef, context))) { + return true; } 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; } - } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.1.ddl.aql ---------------------------------------------------------------------- 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 +}; + http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.2.lib.aql ---------------------------------------------------------------------- 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 http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.3.query.aql ---------------------------------------------------------------------- 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; http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-app/src/test/resources/runtimets/queries/external-library/upperCase/upperCase.4.lib.aql ---------------------------------------------------------------------- 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 http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/upperCase/upperCase.1.adm ---------------------------------------------------------------------- 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 http://git-wip-us.apache.org/repos/asf/asterixdb/blob/bacf0c59/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it.xml ---------------------------------------------------------------------- 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">
