This is an automated email from the ASF dual-hosted git repository. mblow pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 67714e2ac334b6455e977298ac9cd1a65a88d1b7 Author: Dmitry Lychagin <[email protected]> AuthorDate: Fri Jan 29 11:18:58 2021 -0800 [ASTERIXDB-2825][COMP] Improve warning reporting - user model changes: no - storage format changes: no - interface changes: no Details: - Report warnings when parsing user-defined functions Change-Id: Ifd5e4f694c2903dd46245baeba0b866fd7cecb90 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/9786 Integration-Tests: Jenkins <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> Tested-by: Jenkins <[email protected]> --- .../unknown-hint-warning.6.ddl.sqlpp | 37 ++++++++++++++++++++++ .../unknown-hint-warning.7.query.sqlpp | 30 ++++++++++++++++++ .../unknown-hint-warning.7.adm | 1 + .../test/resources/runtimets/testsuite_sqlpp.xml | 2 ++ .../apache/asterix/lang/common/base/IParser.java | 7 ++++ .../asterix/lang/common/parser/FunctionParser.java | 10 ++++-- .../asterix/lang/common/util/FunctionUtil.java | 10 +++--- .../lang/sqlpp/rewrites/SqlppQueryRewriter.java | 2 +- .../asterix-lang-sqlpp/src/main/javacc/SQLPP.jj | 8 ++++- 9 files changed, 99 insertions(+), 8 deletions(-) diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/unknown-hint-warning/unknown-hint-warning.6.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/unknown-hint-warning/unknown-hint-warning.6.ddl.sqlpp new file mode 100644 index 0000000..15d0f69 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/unknown-hint-warning/unknown-hint-warning.6.ddl.sqlpp @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Description : Warning when a hint inside a user-defined function is not recognized + * : Test that the warning is reported when processing CREATE FUNCTION. + * Expected : SUCCESS (with ASX1107 warning) + */ +// requesttype=application/json +// param max-warnings:json=10 + +drop dataverse test if exists; +create dataverse test; + +use test; + +create function f1() { + select value r + from range(1, 4) r + where r /*+ unknown_hint_relexpr_6 */ < 2 +}; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/unknown-hint-warning/unknown-hint-warning.7.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/unknown-hint-warning/unknown-hint-warning.7.query.sqlpp new file mode 100644 index 0000000..2dd8d99 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/unknown-hint-warning/unknown-hint-warning.7.query.sqlpp @@ -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. + */ + +/* + * Description : Warning when a hint inside a user-defined function is not recognized + * : Test that the warning is reported when processing a query that uses that function. + * Expected : SUCCESS (with ASX1107 warning) + */ +// requesttype=application/json +// param max-warnings:json=10 + +use test; + +f1(); \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/unknown-hint-warning/unknown-hint-warning.7.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/unknown-hint-warning/unknown-hint-warning.7.adm new file mode 100644 index 0000000..56a6051 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/unknown-hint-warning/unknown-hint-warning.7.adm @@ -0,0 +1 @@ +1 \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 03db5e8..497202f 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -14143,6 +14143,8 @@ <expected-warn>ASX1107: Unexpected hint: unknown_hint_between. "indexnl", "skip-index", "use-index" expected at this location</expected-warn> <expected-warn>ASX1107: Unexpected hint: unknown_hint_funcall. "indexnl", "range", "skip-index", "use-index" expected at this location</expected-warn> <expected-warn>ASX1107: Unexpected hint: unknown_hint_elsewhere. None expected at this location</expected-warn> + <expected-warn>ASX1107: Unexpected hint: unknown_hint_relexpr_6. "hash-bcast", "indexnl", "skip-index", "use-index" expected at this location</expected-warn> + <expected-warn>ASX1107: Unexpected hint: unknown_hint_relexpr_6. "hash-bcast", "indexnl", "skip-index", "use-index" expected at this location</expected-warn> </compilation-unit> </test-case> <test-case FilePath="warnings"> diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/IParser.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/IParser.java index 86c4b1c..0fe58ff 100644 --- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/IParser.java +++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/IParser.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.asterix.common.exceptions.CompilationException; import org.apache.asterix.common.functions.FunctionSignature; import org.apache.asterix.lang.common.statement.FunctionDecl; +import org.apache.hyracks.api.exceptions.IWarningCollector; import org.apache.hyracks.api.exceptions.Warning; public interface IParser { @@ -37,6 +38,12 @@ public interface IParser { FunctionDecl parseFunctionBody(FunctionSignature signature, List<String> paramNames) throws CompilationException; /** + * Gets the warnings generated during parsing + */ + default void getWarnings(IWarningCollector outWarningCollector) { + } + + /** * Gets the warnings generated during parsing up to the max number argument. */ default void getWarnings(Collection<? super Warning> outWarnings, long maxWarnings) { diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/parser/FunctionParser.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/parser/FunctionParser.java index 52f8a16..d18aa86 100644 --- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/parser/FunctionParser.java +++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/parser/FunctionParser.java @@ -27,6 +27,7 @@ import org.apache.asterix.lang.common.base.IParser; import org.apache.asterix.lang.common.base.IParserFactory; import org.apache.asterix.lang.common.statement.FunctionDecl; import org.apache.asterix.metadata.entities.Function; +import org.apache.hyracks.api.exceptions.IWarningCollector; public class FunctionParser { @@ -40,14 +41,19 @@ public class FunctionParser { return parserFactory.getLanguage(); } - public FunctionDecl getFunctionDecl(Function function) throws CompilationException { + public FunctionDecl getFunctionDecl(Function function, IWarningCollector warningCollector) + throws CompilationException { if (!function.getLanguage().equals(getLanguage())) { throw new CompilationException(ErrorCode.COMPILATION_INCOMPATIBLE_FUNCTION_LANGUAGE, getLanguage(), function.getLanguage()); } IParser parser = parserFactory.createParser(new StringReader(function.getFunctionBody())); try { - return parser.parseFunctionBody(function.getSignature(), function.getParameterNames()); + FunctionDecl functionDecl = parser.parseFunctionBody(function.getSignature(), function.getParameterNames()); + if (warningCollector != null) { + parser.getWarnings(warningCollector); + } + return functionDecl; } catch (CompilationException e) { throw new CompilationException(ErrorCode.COMPILATION_BAD_FUNCTION_DEFINITION, e, function.getSignature(), e.getMessage()); diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java index a33cd61..0592d1e 100644 --- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java +++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java @@ -58,6 +58,7 @@ import org.apache.hyracks.algebricks.common.utils.Triple; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; +import org.apache.hyracks.api.exceptions.IWarningCollector; import org.apache.hyracks.api.exceptions.SourceLocation; public class FunctionUtil { @@ -206,12 +207,13 @@ public class FunctionUtil { * for collecting function calls in the <code>expression</code> * @param functionParser, * for parsing stored functions in the string represetnation. - * @param defaultDataverse + * @param warningCollector + * for reporting warnings encountered during parsing * @throws CompilationException */ public static List<FunctionDecl> retrieveUsedStoredFunctions(MetadataProvider metadataProvider, Expression expression, List<FunctionSignature> declaredFunctions, List<FunctionDecl> inputFunctionDecls, - IFunctionCollector functionCollector, FunctionParser functionParser, DataverseName defaultDataverse) + IFunctionCollector functionCollector, FunctionParser functionParser, IWarningCollector warningCollector) throws CompilationException { if (expression == null) { return Collections.emptyList(); @@ -246,7 +248,7 @@ public class FunctionUtil { continue; } - FunctionDecl functionDecl = functionParser.getFunctionDecl(function); + FunctionDecl functionDecl = functionParser.getFunctionDecl(function, warningCollector); if (functionDecls.contains(functionDecl)) { throw new CompilationException(ErrorCode.COMPILATION_ERROR, functionCall.getSourceLocation(), "Recursive invocation " + functionDecls.get(functionDecls.size() - 1).getSignature() + " <==> " @@ -254,7 +256,7 @@ public class FunctionUtil { } functionDecls.add(functionDecl); functionDecls = retrieveUsedStoredFunctions(metadataProvider, functionDecl.getFuncBody(), declaredFunctions, - functionDecls, functionCollector, functionParser, function.getDataverseName()); + functionDecls, functionCollector, functionParser, warningCollector); } return functionDecls; } diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java index ed56443..755c1df 100644 --- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java +++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java @@ -317,7 +317,7 @@ public class SqlppQueryRewriter implements IQueryRewriter { List<FunctionDecl> usedStoredFunctionDecls = new ArrayList<>(); for (Expression topLevelExpr : topExpr.getDirectlyEnclosedExpressions()) { usedStoredFunctionDecls.addAll(FunctionUtil.retrieveUsedStoredFunctions(metadataProvider, topLevelExpr, - funIds, null, this::getFunctionCalls, functionParser, metadataProvider.getDefaultDataverseName())); + funIds, null, this::getFunctionCalls, functionParser, context.getWarningCollector())); } declaredFunctions.addAll(usedStoredFunctionDecls); if (inlineUdfs && !declaredFunctions.isEmpty()) { diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj index 3d451ee..eee3f8d 100644 --- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj +++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj @@ -205,6 +205,7 @@ import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.core.algebra.expressions.BroadcastExpressionAnnotation; import org.apache.hyracks.algebricks.core.algebra.expressions.IExpressionAnnotation; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; +import org.apache.hyracks.api.exceptions.IWarningCollector; import org.apache.hyracks.api.exceptions.SourceLocation; import org.apache.hyracks.api.exceptions.Warning; import org.apache.hyracks.dataflow.common.data.partition.range.RangeMap; @@ -326,7 +327,6 @@ class SQLPPParser extends ScopeChecker implements IParser { public SQLPPParser(String s) { this(new StringReader(s)); super.setInput(s); - token_source.hintCollector = hintCollector; } public static void main(String args[]) throws ParseException, TokenMgrError, IOException, FileNotFoundException, CompilationException { @@ -408,6 +408,7 @@ class SQLPPParser extends ScopeChecker implements IParser { private <T> T parseImpl(ParseFunction<T> parseFunction) throws CompilationException { warningCollector.clear(); hintCollector.clear(); + token_source.hintCollector = hintCollector; try { return parseFunction.parse(); } catch (SqlppParseException e) { @@ -430,6 +431,11 @@ class SQLPPParser extends ScopeChecker implements IParser { } @Override + public void getWarnings(IWarningCollector outWarningCollector) { + warningCollector.getWarnings(outWarningCollector); + } + + @Override public void getWarnings(Collection<? super Warning> outWarnings, long maxWarnings) { warningCollector.getWarnings(outWarnings, maxWarnings); }
