This is an automated email from the ASF dual-hosted git repository. imaxon pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 404e382c28d50e7008ef1c1d5b7095412f9de5d8 Author: Dmitry Lychagin <[email protected]> AuthorDate: Mon May 3 17:42:16 2021 -0700 [ASTERIXDB-2883][COMP] Improve null handling in UDF calls - user model changes: no - storage format changes: no - interface changes: no - IntroduceDynamicTypeCastForExternalFunctionRule should not apply twice on the same operator - Add null-call handling to ExternalTypeComputer and ExternalScalarJavaFunctionEvaluator Change-Id: I9c58e28f673606c5d0413bdc4cd3ba0c7c20eb8d Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/11306 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> Reviewed-by: Ian Maxon <[email protected]> --- ...duceDynamicTypeCastForExternalFunctionRule.java | 9 ++++- .../type_name/type_name.0.ddl.sqlpp | 20 ++++++++++ .../type_name/type_name.1.lib.sqlpp | 20 ++++++++++ .../type_name/type_name.2.ddl.sqlpp | 25 ++++++++++++ .../type_name/type_name.3.query.sqlpp | 28 +++++++++++++ .../type_name/type_name.4.ddl.sqlpp | 20 ++++++++++ .../external-library/type_name/type_name.3.adm | 1 + .../resources/runtimets/testsuite_it_sqlpp.xml | 5 +++ .../ExternalScalarJavaFunctionEvaluator.java | 45 ++++++++++++++------- .../asterix/external/library/TypeNameFactory.java | 30 ++++++++++++++ .../asterix/external/library/TypeNameFunction.java | 46 ++++++++++++++++++++++ .../functions/ExternalFunctionCompilerUtil.java | 2 +- .../metadata/functions/ExternalTypeComputer.java | 13 ++++-- 13 files changed, 244 insertions(+), 20 deletions(-) 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 d3eb0c2..2c5a07d 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 @@ -115,6 +115,13 @@ public class IntroduceDynamicTypeCastForExternalFunctionRule implements IAlgebra if (op.getOperatorTag() != LogicalOperatorTag.ASSIGN) { return false; } - return op.acceptExpressionTransform(expr -> rewriteFunctionArgs(op, expr, context)); + if (context.checkIfInDontApplySet(this, op)) { + return false; + } + boolean applied = op.acceptExpressionTransform(expr -> rewriteFunctionArgs(op, expr, context)); + if (applied) { + context.addToDontApplySet(this, op); + } + return applied; } } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.0.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.0.ddl.sqlpp new file mode 100644 index 0000000..76cc70d --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.0.ddl.sqlpp @@ -0,0 +1,20 @@ +/* + * 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; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.1.lib.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.1.lib.sqlpp new file mode 100644 index 0000000..b2bf929 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.1.lib.sqlpp @@ -0,0 +1,20 @@ +/* + * 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 java admin admin target/data/externallib/asterix-external-data-testlib.zip diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.2.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.2.ddl.sqlpp new file mode 100644 index 0000000..c7d6768 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.2.ddl.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. + */ +use externallibtest; + +create function typeName(v) returns string + as "org.apache.asterix.external.library.TypeNameFactory" at testlib; + +create function typeNameNullCall(v) returns string + as "org.apache.asterix.external.library.TypeNameFactory" at testlib with {"null-call": true}; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.3.query.sqlpp new file mode 100644 index 0000000..2542d55 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.3.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. + */ +use externallibtest; + +{ + "missing": typeName(missing) is missing, + "missing_nullcall": typeNameNullCall(missing), + "null": typeName(null) is null, + "null_nullcall": typeNameNullCall(null), + "boolean": typeName(true), + "string": typeName("x") +} diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.4.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.4.ddl.sqlpp new file mode 100644 index 0000000..2b27030 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-library/type_name/type_name.4.ddl.sqlpp @@ -0,0 +1,20 @@ +/* + * 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; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/type_name/type_name.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/type_name/type_name.3.adm new file mode 100644 index 0000000..a72c46c --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/external-library/type_name/type_name.3.adm @@ -0,0 +1 @@ +{ "missing": true, "missing_nullcall": "missing", "null": true, "null_nullcall": "null", "boolean": "boolean", "string": "string" } \ No newline at end of file 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 28cbabb..6c67216 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 @@ -42,6 +42,11 @@ </compilation-unit> </test-case> <test-case FilePath="external-library"> + <compilation-unit name="type_name"> + <output-dir compare="Text">type_name</output-dir> + </compilation-unit> + </test-case> + <test-case FilePath="external-library"> <compilation-unit name="type_validation"> <output-dir compare="Text">type_validation</output-dir> </compilation-unit> diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalScalarJavaFunctionEvaluator.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalScalarJavaFunctionEvaluator.java index 33b0369..35d55f7 100755 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalScalarJavaFunctionEvaluator.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalScalarJavaFunctionEvaluator.java @@ -19,7 +19,7 @@ package org.apache.asterix.external.library; -import java.io.IOException; +import static org.apache.asterix.om.types.EnumDeserializer.ATYPETAGDESERIALIZER; import org.apache.asterix.common.exceptions.ErrorCode; import org.apache.asterix.common.exceptions.RuntimeDataException; @@ -27,7 +27,9 @@ import org.apache.asterix.common.metadata.DataverseName; import org.apache.asterix.external.api.IExternalScalarFunction; import org.apache.asterix.external.api.IFunctionFactory; import org.apache.asterix.om.functions.IExternalFunctionInfo; +import org.apache.asterix.om.types.ATypeTag; import org.apache.asterix.om.types.IAType; +import org.apache.asterix.runtime.evaluators.functions.PointableHelper; import org.apache.hyracks.algebricks.runtime.base.IEvaluatorContext; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluatorFactory; import org.apache.hyracks.api.exceptions.HyracksDataException; @@ -71,23 +73,36 @@ class ExternalScalarJavaFunctionEvaluator extends ExternalScalarFunctionEvaluato @Override public void evaluate(IFrameTupleReference tuple, IPointable result) throws HyracksDataException { try { - setArguments(tuple); - resultBuffer.reset(); - externalFunctionInstance.evaluate(functionHelper); - if (!functionHelper.isValidResult()) { - throw new RuntimeDataException(ErrorCode.EXTERNAL_UDF_RESULT_TYPE_ERROR); + boolean nullCall = finfo.getNullCall(); + boolean hasNullArg = false; + for (int i = 0; i < argEvals.length; i++) { + argEvals[i].evaluate(tuple, inputVal); + if (!nullCall) { + byte[] inputValBytes = inputVal.getByteArray(); + int inputValStartOffset = inputVal.getStartOffset(); + ATypeTag typeTag = ATYPETAGDESERIALIZER.deserialize(inputValBytes[inputValStartOffset]); + if (typeTag == ATypeTag.MISSING) { + PointableHelper.setMissing(result); + return; + } else if (typeTag == ATypeTag.NULL) { + hasNullArg = true; + } + } + functionHelper.setArgument(i, inputVal); + } + if (!nullCall && hasNullArg) { + PointableHelper.setNull(result); + } else { + resultBuffer.reset(); + externalFunctionInstance.evaluate(functionHelper); + if (!functionHelper.isValidResult()) { + throw new RuntimeDataException(ErrorCode.EXTERNAL_UDF_RESULT_TYPE_ERROR); + } + result.set(resultBuffer.getByteArray(), resultBuffer.getStartOffset(), resultBuffer.getLength()); + functionHelper.reset(); } - result.set(resultBuffer.getByteArray(), resultBuffer.getStartOffset(), resultBuffer.getLength()); - functionHelper.reset(); } catch (Exception e) { throw HyracksDataException.create(e); } } - - public void setArguments(IFrameTupleReference tuple) throws IOException { - for (int i = 0; i < argEvals.length; i++) { - argEvals[i].evaluate(tuple, inputVal); - functionHelper.setArgument(i, inputVal); - } - } } diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/TypeNameFactory.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/TypeNameFactory.java new file mode 100644 index 0000000..335c383 --- /dev/null +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/TypeNameFactory.java @@ -0,0 +1,30 @@ +/* + * 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. + */ + +package org.apache.asterix.external.library; + +import org.apache.asterix.external.api.IExternalScalarFunction; +import org.apache.asterix.external.api.IFunctionFactory; + +public class TypeNameFactory implements IFunctionFactory { + @Override + public IExternalScalarFunction getExternalFunction() { + return new TypeNameFunction(); + } +} diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/TypeNameFunction.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/TypeNameFunction.java new file mode 100644 index 0000000..1f03fcc --- /dev/null +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/TypeNameFunction.java @@ -0,0 +1,46 @@ +/* + * 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. + */ + +package org.apache.asterix.external.library; + +import org.apache.asterix.external.api.IExternalScalarFunction; +import org.apache.asterix.external.api.IFunctionHelper; +import org.apache.asterix.external.library.java.base.JString; + +public class TypeNameFunction implements IExternalScalarFunction { + + private JString result; + + @Override + public void deinitialize() { + // nothing to do here + } + + @Override + public void evaluate(IFunctionHelper functionHelper) throws Exception { + String arg0TypeName = functionHelper.getArgument(0).getIAType().getTypeName(); + result.setValue(arg0TypeName); + functionHelper.setResult(result); + } + + @Override + public void initialize(IFunctionHelper functionHelper) { + result = (JString) functionHelper.getResultObject(); + } +} 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 6751171..5fd96a6 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 @@ -69,7 +69,7 @@ public class ExternalFunctionCompilerUtil { IAType returnType = getType(function.getReturnType(), metadataProvider); - IResultTypeComputer typeComputer = new ExternalTypeComputer(returnType, paramTypes); + IResultTypeComputer typeComputer = new ExternalTypeComputer(returnType, paramTypes, function.getNullCall()); ExternalFunctionLanguage lang = getExternalFunctionLanguage(function.getLanguage()); 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 4a000a3..99d8ea2 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 @@ -33,8 +33,9 @@ import org.apache.hyracks.api.exceptions.SourceLocation; public class ExternalTypeComputer extends AbstractResultTypeComputer { - private IAType resultType; - private List<IAType> paramPrimeTypes; + private final IAType resultType; + private final List<IAType> paramPrimeTypes; + private final boolean nullCall; @Override protected void checkArgType(FunctionIdentifier funcId, int argIndex, IAType type, SourceLocation sourceLoc) @@ -47,14 +48,20 @@ public class ExternalTypeComputer extends AbstractResultTypeComputer { } } - public ExternalTypeComputer(IAType resultPrimeType, List<IAType> paramPrimeTypes) { + public ExternalTypeComputer(IAType resultPrimeType, List<IAType> paramPrimeTypes, boolean nullCall) { this.resultType = resultPrimeType.getTypeTag() == ATypeTag.ANY ? resultPrimeType : AUnionType.createUnknownableType(resultPrimeType); this.paramPrimeTypes = paramPrimeTypes; + this.nullCall = nullCall; } @Override protected IAType getResultType(ILogicalExpression expr, IAType... strippedInputTypes) { return resultType; } + + @Override + protected boolean propagateNullAndMissing() { + return !nullCall; + } }
