Xikui Wang has submitted this change and it was merged. ( https://asterix-gerrit.ics.uci.edu/3270 )
Change subject: [NO ISSUE][FUN] Fix type inference and casting in UDFs ...................................................................... [NO ISSUE][FUN] Fix type inference and casting in UDFs - user model changes: no - storage format changes: no - interface changes: no Details: The current UDF framework handles the argument types in a sloppy way. It takes in the arguments and reads them as the expected data types in the configuration. This could cause exception at runtime when passing in arguments with unexpected datatypes. When setting the arguments, it did a type casting for numeric values only to make sure the int64 from query interface can be evaluated properly. However, this is not robust enough. This patch fixes the type inference for UDFs during the complation time. The ExternalTypeComputer is refactored to return defined data type, and meanwhile checks the argument data types. Also, the IntroduceDynamicTypeCastForExternalFunctionRule is modified to cover the type castings for data types besides record type. Change-Id: I40506fcca3cd8f14bbd6412359683433256c4c1f Reviewed-on: https://asterix-gerrit.ics.uci.edu/3270 Tested-by: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Xikui Wang <[email protected]> --- M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastForExternalFunctionRule.java C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.1.ddl.sqlpp C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.2.lib.sqlpp C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.3.query.sqlpp C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.6.lib.sqlpp C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.7.ddl.sqlpp C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.query.sqlpp C asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.query.sqlpp R asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.6.lib.sqlpp R asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.7.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/my_array_sum/my_array_sum.1.adm A asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mysum/mysum.2.adm M asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/validate-default-library/validate-default-library.1.adm M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunctionProvider.java C asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFactory.java C asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFunction.java R asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFactory.java R asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFunction.java M asterixdb/asterix-external-data/src/test/resources/library_descriptor.xml M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtil.java M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalTypeComputer.java M asterixdb/asterix-metadata/src/test/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtilTest.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/ExternalFunctionInfo.java A asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/my_array_sum/my_array_sum.1.adm A asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/mysum/mysum.2.adm M asterixdb/asterix-server/src/test/resources/integrationts/library/testsuite.xml 28 files changed, 124 insertions(+), 188 deletions(-) Approvals: Jenkins: Verified; Verified Xikui Wang: Looks good to me, approved Objections: Anon. E. Moose (1000171): Violations found Jenkins: Violations found 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 97e2174..a00933a 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 @@ -24,6 +24,7 @@ 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.ATypeTag; import org.apache.asterix.om.types.AUnionType; import org.apache.asterix.om.types.IAType; import org.apache.asterix.om.utils.NonTaggedFormatUtil; @@ -71,36 +72,37 @@ if (BuiltinFunctions.getBuiltinFunctionIdentifier(funcCallExpr.getFunctionIdentifier()) != null) { return changed; } - IAType inputRecordType; - ARecordType requiredRecordType; - for (int iter1 = 0; iter1 < funcCallExpr.getArguments().size(); iter1++) { - Mutable<ILogicalExpression> argExpr = funcCallExpr.getArguments().get(iter1); - inputRecordType = (IAType) op.computeOutputTypeEnvironment(context).getType(argExpr.getValue()); - if (!(((ExternalScalarFunctionInfo) funcCallExpr.getFunctionInfo()).getArgumenTypes() - .get(iter1) instanceof ARecordType)) { - continue; + IAType inputType; + IAType reqArgType; + boolean castFlag; + for (int i = 0; i < funcCallExpr.getArguments().size(); i++) { + Mutable<ILogicalExpression> argExpr = funcCallExpr.getArguments().get(i); + inputType = (IAType) op.computeOutputTypeEnvironment(context).getType(argExpr.getValue()); + reqArgType = ((ExternalScalarFunctionInfo) funcCallExpr.getFunctionInfo()).getArgumentTypes().get(i); + + if (reqArgType.getTypeTag() == ATypeTag.OBJECT) { + castFlag = !IntroduceDynamicTypeCastRule.compatible((ARecordType) reqArgType, inputType, + argExpr.getValue().getSourceLocation()); + } else { + castFlag = !reqArgType.equals(inputType); } - 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 (NonTaggedFormatUtil.isOptional(inputType)) { /** while-loop for the case there is a nested multi-level union */ - inputRecordType = ((AUnionType) inputRecordType).getActualType(); + inputType = ((AUnionType) inputType).getActualType(); checkUnknown = true; } - boolean castFlag = !IntroduceDynamicTypeCastRule.compatible(requiredRecordType, inputRecordType, - argExpr.getValue().getSourceLocation()); if (castFlag || checkUnknown) { AbstractFunctionCallExpression castFunc = new ScalarFunctionCallExpression(FunctionUtil.getFunctionInfo(BuiltinFunctions.CAST_TYPE)); castFunc.setSourceLocation(argExpr.getValue().getSourceLocation()); castFunc.getArguments().add(argExpr); - TypeCastUtils.setRequiredAndInputTypes(castFunc, requiredRecordType, inputRecordType); - funcCallExpr.getArguments().set(iter1, new MutableObject<>(castFunc)); + TypeCastUtils.setRequiredAndInputTypes(castFunc, reqArgType, inputType); + funcCallExpr.getArguments().set(i, new MutableObject<>(castFunc)); changed = true; } } diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.1.ddl.sqlpp similarity index 69% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.1.ddl.sqlpp index 5202093..76cc70d 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.1.ddl.sqlpp @@ -16,16 +16,5 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.asterix.external.library; - -import org.apache.asterix.external.api.IExternalScalarFunction; -import org.apache.asterix.external.api.IFunctionFactory; - -public class SumFactory implements IFunctionFactory { - - @Override - public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); - } - -} +DROP DATAVERSE externallibtest if exists; +CREATE DATAVERSE externallibtest; diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.2.lib.sqlpp similarity index 69% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.2.lib.sqlpp index 5202093..d1e0e87 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.2.lib.sqlpp @@ -16,16 +16,4 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.asterix.external.library; - -import org.apache.asterix.external.api.IExternalScalarFunction; -import org.apache.asterix.external.api.IFunctionFactory; - -public class SumFactory implements IFunctionFactory { - - @Override - public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); - } - -} +install externallibtest testlib target/data/externallib/asterix-external-data-testlib.zip \ No newline at end of file diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.3.query.sqlpp similarity index 69% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.3.query.sqlpp index 5202093..f9ebecd 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.3.query.sqlpp @@ -16,16 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.asterix.external.library; +use externallibtest; -import org.apache.asterix.external.api.IExternalScalarFunction; -import org.apache.asterix.external.api.IFunctionFactory; - -public class SumFactory implements IFunctionFactory { - - @Override - public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); - } - -} +testlib#my_array_sum([8.9, 9.7, 1]); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.lib.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.6.lib.sqlpp similarity index 100% copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.lib.sqlpp copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.6.lib.sqlpp diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.7.ddl.sqlpp similarity index 100% copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.ddl.sqlpp copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/my_array_sum/my_array_sum.7.ddl.sqlpp diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.query.sqlpp similarity index 69% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.query.sqlpp index 5202093..9fd6f94 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.query.sqlpp @@ -16,16 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.asterix.external.library; +use externallibtest; -import org.apache.asterix.external.api.IExternalScalarFunction; -import org.apache.asterix.external.api.IFunctionFactory; - -public class SumFactory implements IFunctionFactory { - - @Override - public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); - } - -} +testlib#mysum(9.7, 4); diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.query.sqlpp similarity index 69% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.query.sqlpp index 5202093..776f976 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.query.sqlpp @@ -16,16 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.asterix.external.library; +use externallibtest; -import org.apache.asterix.external.api.IExternalScalarFunction; -import org.apache.asterix.external.api.IFunctionFactory; - -public class SumFactory implements IFunctionFactory { - - @Override - public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); - } - -} +testlib#mysum("3",4); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.lib.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.6.lib.sqlpp similarity index 100% rename from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.4.lib.sqlpp rename to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.6.lib.sqlpp diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.7.ddl.sqlpp similarity index 100% rename from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.5.ddl.sqlpp rename to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/mysum/mysum.7.ddl.sqlpp diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/my_array_sum/my_array_sum.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/my_array_sum/my_array_sum.1.adm new file mode 100644 index 0000000..25bf17f --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/my_array_sum/my_array_sum.1.adm @@ -0,0 +1 @@ +18 \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mysum/mysum.2.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mysum/mysum.2.adm new file mode 100644 index 0000000..ca7bf83 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/mysum/mysum.2.adm @@ -0,0 +1 @@ +13 \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/validate-default-library/validate-default-library.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/validate-default-library/validate-default-library.1.adm index 13de599..ee90703 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/validate-default-library/validate-default-library.1.adm +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/validate-default-library/validate-default-library.1.adm @@ -5,7 +5,8 @@ { "Function": { "DataverseName": "externallibtest", "Name": "testlib#fnameDetector", "Arity": "1", "Params": [ "InputRecordType" ], "ReturnType": "DetectResultType", "Definition": "org.apache.asterix.external.library.KeywordsDetectorFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } { "Function": { "DataverseName": "externallibtest", "Name": "testlib#getCapital", "Arity": "1", "Params": [ "ASTRING" ], "ReturnType": "CountryCapitalType", "Definition": "org.apache.asterix.external.library.CapitalFinderFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } { "Function": { "DataverseName": "externallibtest", "Name": "testlib#lnameDetector", "Arity": "1", "Params": [ "InputRecordType" ], "ReturnType": "DetectResultType", "Definition": "org.apache.asterix.external.library.KeywordsDetectorFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } -{ "Function": { "DataverseName": "externallibtest", "Name": "testlib#mysum", "Arity": "2", "Params": [ "AINT32", "AINT32" ], "ReturnType": "AINT32", "Definition": "org.apache.asterix.external.library.SumFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } +{ "Function": { "DataverseName": "externallibtest", "Name": "testlib#my_array_sum", "Arity": "1", "Params": [ "[AINT32]" ], "ReturnType": "AINT32", "Definition": "org.apache.asterix.external.library.MyArraySumFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } +{ "Function": { "DataverseName": "externallibtest", "Name": "testlib#mysum", "Arity": "2", "Params": [ "AINT32", "AINT32" ], "ReturnType": "AINT32", "Definition": "org.apache.asterix.external.library.MySumFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } { "Function": { "DataverseName": "externallibtest", "Name": "testlib#parseTweet", "Arity": "1", "Params": [ "TweetInputType" ], "ReturnType": "TweetOutputType", "Definition": "org.apache.asterix.external.library.ParseTweetFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } { "Function": { "DataverseName": "externallibtest", "Name": "testlib#toUpper", "Arity": "1", "Params": [ "TextType" ], "ReturnType": "TextType", "Definition": "org.apache.asterix.external.library.UpperCaseFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } { "Function": { "DataverseName": "externallibtest", "Name": "testlib#typeValidation", "Arity": "11", "Params": [ "AINT32", "AFLOAT", "ASTRING", "ADouble", "ABoolean", "APoint", "ADate", "ADatetime", "ALine", "ACircle", "ARectangle" ], "ReturnType": "AString", "Definition": "org.apache.asterix.external.library.TypeValidationFunctionFactory", "Language": "JAVA", "Kind": "SCALAR", "Dependencies": [ [ ], [ ] ] } } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml index 2121aae..71d0503 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_it_sqlpp.xml @@ -31,6 +31,12 @@ <test-case FilePath="external-library"> <compilation-unit name="mysum"> <output-dir compare="Text">mysum</output-dir> + <expected-error>Type mismatch: function testlib#mysum expects its 1st input parameter to be of type integer</expected-error> + </compilation-unit> + </test-case> + <test-case FilePath="external-library"> + <compilation-unit name="my_array_sum"> + <output-dir compare="Text">my_array_sum</output-dir> </compilation-unit> </test-case> <test-case FilePath="external-library"> diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java index c5b9ad6..33fff14 100755 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java @@ -28,9 +28,6 @@ import org.apache.asterix.external.api.IFunctionFactory; import org.apache.asterix.external.api.IFunctionHelper; import org.apache.asterix.om.functions.IExternalFunctionInfo; -import org.apache.asterix.om.types.ATypeTag; -import org.apache.asterix.om.types.EnumDeserializer; -import org.apache.asterix.om.types.hierachy.ATypeHierarchy; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluator; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluatorFactory; @@ -45,11 +42,10 @@ protected final IExternalFunctionInfo finfo; protected final IFunctionFactory externalFunctionFactory; - protected final IExternalFunction externalFunction; + protected final IExternalFunction externalFunctionInstance; protected final IScalarEvaluatorFactory[] evaluatorFactories; protected final IPointable inputVal = new VoidPointable(); protected final ArrayBackedValueStorage resultBuffer = new ArrayBackedValueStorage(); - protected final ArrayBackedValueStorage castBuffer = new ArrayBackedValueStorage(); protected final IScalarEvaluator[] argumentEvaluators; protected final JavaFunctionHelper functionHelper; @@ -75,7 +71,7 @@ try { clazz = Class.forName(classname, true, libraryClassLoader); externalFunctionFactory = (IFunctionFactory) clazz.newInstance(); - externalFunction = externalFunctionFactory.getExternalFunction(); + externalFunctionInstance = externalFunctionFactory.getExternalFunction(); } catch (Exception e) { throw new RuntimeDataException(ErrorCode.LIBRARY_EXTERNAL_FUNCTION_UNABLE_TO_LOAD_CLASS, e, classname); } @@ -84,30 +80,18 @@ public void setArguments(IFrameTupleReference tuple) throws AlgebricksException, IOException { for (int i = 0; i < evaluatorFactories.length; i++) { argumentEvaluators[i].evaluate(tuple, inputVal); - - // Type-cast the source array based on the input type that this function wants to receive. - ATypeTag targetTypeTag = finfo.getArgumentList().get(i).getTypeTag(); - ATypeTag sourceTypeTag = EnumDeserializer.ATYPETAGDESERIALIZER - .deserialize(inputVal.getByteArray()[inputVal.getStartOffset()]); - if (sourceTypeTag != targetTypeTag) { - castBuffer.reset(); - ATypeHierarchy.convertNumericTypeByteArray(inputVal.getByteArray(), inputVal.getStartOffset(), - inputVal.getLength(), targetTypeTag, castBuffer.getDataOutput(), true); - functionHelper.setArgument(i, castBuffer); - } else { - functionHelper.setArgument(i, inputVal); - } + functionHelper.setArgument(i, inputVal); } } @Override public void deinitialize() { - externalFunction.deinitialize(); + externalFunctionInstance.deinitialize(); } @Override public void initialize(IFunctionHelper functionHelper) throws Exception { - externalFunction.initialize(functionHelper); + externalFunctionInstance.initialize(functionHelper); } } diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunctionProvider.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunctionProvider.java index 05124dd..83b60f1 100755 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunctionProvider.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunctionProvider.java @@ -77,7 +77,7 @@ public void evaluate(IFunctionHelper argumentProvider) throws HyracksDataException { try { resultBuffer.reset(); - ((IExternalScalarFunction) externalFunction).evaluate(argumentProvider); + ((IExternalScalarFunction) externalFunctionInstance).evaluate(argumentProvider); if (!argumentProvider.isValidResult()) { throw new RuntimeDataException(ErrorCode.EXTERNAL_UDF_RESULT_TYPE_ERROR); } diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFactory.java similarity index 91% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java copy to asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFactory.java index 5202093..3f8c945 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFactory.java @@ -21,11 +21,11 @@ import org.apache.asterix.external.api.IExternalScalarFunction; import org.apache.asterix.external.api.IFunctionFactory; -public class SumFactory implements IFunctionFactory { +public class MyArraySumFactory implements IFunctionFactory { @Override public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); + return new MyArraySumFunction(); } } diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFunction.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFunction.java similarity index 78% copy from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFunction.java copy to asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFunction.java index e16d779..67c2889 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFunction.java +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MyArraySumFunction.java @@ -21,8 +21,9 @@ import org.apache.asterix.external.api.IExternalScalarFunction; import org.apache.asterix.external.api.IFunctionHelper; import org.apache.asterix.external.library.java.base.JInt; +import org.apache.asterix.external.library.java.base.JOrderedList; -public class SumFunction implements IExternalScalarFunction { +public class MyArraySumFunction implements IExternalScalarFunction { private JInt result; @@ -33,9 +34,12 @@ @Override public void evaluate(IFunctionHelper functionHelper) throws Exception { - int arg0 = ((JInt) functionHelper.getArgument(0)).getValue(); - int arg1 = ((JInt) functionHelper.getArgument(1)).getValue(); - result.setValue(arg0 + arg1); + JOrderedList arg0 = (JOrderedList) (functionHelper.getArgument(0)); + int sum = 0; + for (int iter1 = 0; iter1 < arg0.size(); iter1++) { + sum += ((JInt) arg0.getValue().get(iter1)).getValue(); + } + result.setValue(sum); functionHelper.setResult(result); } diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFactory.java similarity index 92% rename from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java rename to asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFactory.java index 5202093..0e071e4 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFactory.java +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFactory.java @@ -21,11 +21,11 @@ import org.apache.asterix.external.api.IExternalScalarFunction; import org.apache.asterix.external.api.IFunctionFactory; -public class SumFactory implements IFunctionFactory { +public class MySumFactory implements IFunctionFactory { @Override public IExternalScalarFunction getExternalFunction() { - return new SumFunction(); + return new MySumFunction(); } } diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFunction.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFunction.java similarity index 96% rename from asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFunction.java rename to asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFunction.java index e16d779..26abee8 100644 --- a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/SumFunction.java +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/MySumFunction.java @@ -22,7 +22,7 @@ import org.apache.asterix.external.api.IFunctionHelper; import org.apache.asterix.external.library.java.base.JInt; -public class SumFunction implements IExternalScalarFunction { +public class MySumFunction implements IExternalScalarFunction { private JInt result; diff --git a/asterixdb/asterix-external-data/src/test/resources/library_descriptor.xml b/asterixdb/asterix-external-data/src/test/resources/library_descriptor.xml index acbc003..720c003 100644 --- a/asterixdb/asterix-external-data/src/test/resources/library_descriptor.xml +++ b/asterixdb/asterix-external-data/src/test/resources/library_descriptor.xml @@ -73,7 +73,15 @@ <function_type>SCALAR</function_type> <argument_type>AINT32,AINT32</argument_type> <return_type>AINT32</return_type> - <definition>org.apache.asterix.external.library.SumFactory + <definition>org.apache.asterix.external.library.MySumFactory + </definition> + </libraryFunction> + <libraryFunction> + <name>my_array_sum</name> + <function_type>SCALAR</function_type> + <argument_type>[AINT32]</argument_type> + <return_type>AINT32</return_type> + <definition>org.apache.asterix.external.library.MyArraySumFactory </definition> </libraryFunction> <libraryFunction> diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtil.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtil.java index 01d0af7..e203c36 100644 --- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtil.java +++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtil.java @@ -29,27 +29,23 @@ import org.apache.asterix.metadata.entities.Datatype; import org.apache.asterix.metadata.entities.Function; import org.apache.asterix.om.typecomputer.base.IResultTypeComputer; -import org.apache.asterix.om.typecomputer.impl.ABinaryTypeComputer; -import org.apache.asterix.om.typecomputer.impl.ADoubleTypeComputer; -import org.apache.asterix.om.typecomputer.impl.AFloatTypeComputer; -import org.apache.asterix.om.typecomputer.impl.AInt32TypeComputer; -import org.apache.asterix.om.typecomputer.impl.AStringTypeComputer; import org.apache.asterix.om.types.AOrderedListType; import org.apache.asterix.om.types.AUnorderedListType; import org.apache.asterix.om.types.BuiltinType; import org.apache.asterix.om.types.IAType; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; -import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression.FunctionKind; -import org.apache.hyracks.algebricks.core.algebra.expressions.IVariableTypeEnvironment; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.algebricks.core.algebra.functions.IFunctionInfo; -import org.apache.hyracks.algebricks.core.algebra.metadata.IMetadataProvider; public class ExternalFunctionCompilerUtil { private static Pattern orderedListPattern = Pattern.compile("\\[*\\]"); private static Pattern unorderedListPattern = Pattern.compile("[{{*}}]"); + + private ExternalFunctionCompilerUtil() { + // do nothing + } public static IFunctionInfo getExternalFunctionInfo(MetadataTransactionContext txnCtx, Function function) throws AlgebricksException { @@ -72,24 +68,27 @@ throws AlgebricksException { FunctionIdentifier fid = new FunctionIdentifier(function.getDataverseName(), function.getName(), function.getArity()); - IResultTypeComputer typeComputer = getResultTypeComputer(txnCtx, function); - List<IAType> arguments = new ArrayList<>(); - IAType returnType; - List<String> argumentTypes = function.getArguments(); - for (String argumentType : argumentTypes) { - arguments.add(getTypeInfo(argumentType, txnCtx, function)); + List<IAType> argumentTypes = new ArrayList<>(); + IAType returnType = getTypeInfo(function.getReturnType(), txnCtx, function);; + for (String argumentType : function.getArguments()) { + argumentTypes.add(getTypeInfo(argumentType, txnCtx, function)); } - - returnType = getTypeInfo(function.getReturnType(), txnCtx, function); + IResultTypeComputer typeComputer = new ExternalTypeComputer(returnType, argumentTypes); return new ExternalScalarFunctionInfo(fid.getNamespace(), fid.getName(), fid.getArity(), returnType, - function.getFunctionBody(), function.getLanguage(), arguments, typeComputer); + function.getFunctionBody(), function.getLanguage(), argumentTypes, typeComputer); } private static IAType getTypeInfo(String paramType, MetadataTransactionContext txnCtx, Function function) throws AlgebricksException { - if (paramType.equalsIgnoreCase(BuiltinType.AINT32.getDisplayName())) { + if (paramType.equalsIgnoreCase(BuiltinType.AINT8.getDisplayName())) { + return BuiltinType.AINT8; + } else if (paramType.equalsIgnoreCase(BuiltinType.AINT16.getDisplayName())) { + return BuiltinType.AINT16; + } else if (paramType.equalsIgnoreCase(BuiltinType.AINT32.getDisplayName())) { return BuiltinType.AINT32; + } else if (paramType.equalsIgnoreCase(BuiltinType.AINT64.getDisplayName())) { + return BuiltinType.AINT64; } else if (paramType.equalsIgnoreCase(BuiltinType.AFLOAT.getDisplayName())) { return BuiltinType.AFLOAT; } else if (paramType.equalsIgnoreCase(BuiltinType.ASTRING.getDisplayName())) { @@ -142,59 +141,6 @@ } } return null; - } - - private static IResultTypeComputer getResultTypeComputer(final MetadataTransactionContext txnCtx, - final Function function) throws AlgebricksException { - - final IAType type = getTypeInfo(function.getReturnType(), txnCtx, function); - switch (type.getTypeTag()) { - case INTEGER: - return AInt32TypeComputer.INSTANCE; - case FLOAT: - return AFloatTypeComputer.INSTANCE; - case DOUBLE: - return ADoubleTypeComputer.INSTANCE; - case BINARY: - return ABinaryTypeComputer.INSTANCE; - case STRING: - return AStringTypeComputer.INSTANCE; - case ARRAY: - return new ExternalTypeComputer() { - private static final long serialVersionUID = 1L; - - @Override - public IAType computeType(ILogicalExpression expression, IVariableTypeEnvironment env, - IMetadataProvider<?, ?> metadataProvider) throws AlgebricksException { - - return new AOrderedListType(((AOrderedListType) type).getItemType(), - ((AOrderedListType) type).getItemType().getTypeName()); - } - - }; - case MULTISET: - return new ExternalTypeComputer() { - private static final long serialVersionUID = 1L; - - @Override - public IAType computeType(ILogicalExpression expression, IVariableTypeEnvironment env, - IMetadataProvider<?, ?> metadataProvider) throws AlgebricksException { - return new AUnorderedListType(((AUnorderedListType) type).getItemType(), type.getTypeName()); - } - - }; - default: - return new ExternalTypeComputer() { - private static final long serialVersionUID = 1L; - - @Override - public IAType computeType(ILogicalExpression expression, IVariableTypeEnvironment env, - IMetadataProvider<?, ?> mp) throws AlgebricksException { - return type; - } - }; - } - } private static IFunctionInfo getUnnestFunctionInfo(MetadataTransactionContext txnCtx, Function function) { diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalTypeComputer.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalTypeComputer.java index d50f4cb..2f5742e 100644 --- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalTypeComputer.java +++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/functions/ExternalTypeComputer.java @@ -18,10 +18,40 @@ */ package org.apache.asterix.metadata.functions; -import java.io.Serializable; +import java.util.List; -import org.apache.asterix.om.typecomputer.base.IResultTypeComputer; +import org.apache.asterix.om.exceptions.TypeMismatchException; +import org.apache.asterix.om.typecomputer.base.AbstractResultTypeComputer; +import org.apache.asterix.om.types.IAType; +import org.apache.asterix.om.types.hierachy.ATypeHierarchy; +import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; +import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; +import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; +import org.apache.hyracks.api.exceptions.SourceLocation; -public interface ExternalTypeComputer extends IResultTypeComputer, Serializable { +public class ExternalTypeComputer extends AbstractResultTypeComputer { + private IAType resultType; + private List<IAType> argTypes; + + @Override + protected void checkArgType(FunctionIdentifier funcId, int argIndex, IAType type, SourceLocation sourceLoc) + throws AlgebricksException { + IAType reqArgType = argTypes.get(argIndex); + if (!type.equals(argTypes.get(argIndex)) + && !ATypeHierarchy.isCompatible(type.getTypeTag(), reqArgType.getTypeTag())) { + throw new TypeMismatchException(sourceLoc, funcId, argIndex, type.getTypeTag(), + argTypes.get(argIndex).getTypeTag()); + } + } + + public ExternalTypeComputer(IAType resultType, List<IAType> argTypes) { + this.resultType = resultType; + this.argTypes = argTypes; + } + + @Override + protected IAType getResultType(ILogicalExpression expr, IAType... strippedInputTypes) { + return resultType; + } } diff --git a/asterixdb/asterix-metadata/src/test/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtilTest.java b/asterixdb/asterix-metadata/src/test/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtilTest.java index b4bae90..78145c1 100644 --- a/asterixdb/asterix-metadata/src/test/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtilTest.java +++ b/asterixdb/asterix-metadata/src/test/java/org/apache/asterix/metadata/functions/ExternalFunctionCompilerUtilTest.java @@ -42,11 +42,9 @@ // when ExternalScalarFunctionInfo info = (ExternalScalarFunctionInfo) ExternalFunctionCompilerUtil.getExternalFunctionInfo(txnCtx, function); - IAType type = info.getResultTypeComputer().computeType(null, null, null); // then IAType expectedType = new AUnorderedListType(BuiltinType.ASTRING, "AUnorderedList"); Assert.assertEquals(expectedType, info.getReturnType()); - Assert.assertEquals(expectedType, type); } } diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/ExternalFunctionInfo.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/ExternalFunctionInfo.java index af1bc7c..2c80470 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/ExternalFunctionInfo.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/ExternalFunctionInfo.java @@ -50,7 +50,7 @@ return rtc; } - public List<IAType> getArgumenTypes() { + public List<IAType> getArgumentTypes() { return argumentTypes; } diff --git a/asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/my_array_sum/my_array_sum.1.adm b/asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/my_array_sum/my_array_sum.1.adm new file mode 100644 index 0000000..25bf17f --- /dev/null +++ b/asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/my_array_sum/my_array_sum.1.adm @@ -0,0 +1 @@ +18 \ No newline at end of file diff --git a/asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/mysum/mysum.2.adm b/asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/mysum/mysum.2.adm new file mode 100644 index 0000000..ca7bf83 --- /dev/null +++ b/asterixdb/asterix-server/src/test/resources/integrationts/library/results/library-functions/mysum/mysum.2.adm @@ -0,0 +1 @@ +13 \ No newline at end of file diff --git a/asterixdb/asterix-server/src/test/resources/integrationts/library/testsuite.xml b/asterixdb/asterix-server/src/test/resources/integrationts/library/testsuite.xml index babf911..76b74ef 100644 --- a/asterixdb/asterix-server/src/test/resources/integrationts/library/testsuite.xml +++ b/asterixdb/asterix-server/src/test/resources/integrationts/library/testsuite.xml @@ -28,6 +28,12 @@ <test-case FilePath="library-functions"> <compilation-unit name="mysum"> <output-dir compare="Text">mysum</output-dir> + <expected-error>Type mismatch: function testlib#mysum expects its 1st input parameter to be of type integer</expected-error> + </compilation-unit> + </test-case> + <test-case FilePath="external-library"> + <compilation-unit name="my_array_sum"> + <output-dir compare="Text">my_array_sum</output-dir> </compilation-unit> </test-case> <test-case FilePath="library-functions"> -- To view, visit https://asterix-gerrit.ics.uci.edu/3270 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I40506fcca3cd8f14bbd6412359683433256c4c1f Gerrit-Change-Number: 3270 Gerrit-PatchSet: 10 Gerrit-Owner: Xikui Wang <[email protected]> Gerrit-Reviewer: Anon. E. Moose (1000171) Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Xikui Wang <[email protected]>
