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 0eeeed149baacba6751c422ea0e4324b268f5cf4 Author: Dmitry Lychagin <[email protected]> AuthorDate: Mon Jun 15 18:45:14 2020 -0700 [ASTERIXDB-2750][COMP] Incorrect result with unnest and join - user model changes: no - storage format changes: no - interface changes: no Details: - Fix incorrect result when FROM clause contains simple unnest (",") followed by JOIN Change-Id: Icfd174ad099aaf24f9553349b3d417d64f9c70b5 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/6824 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Till Westmann <[email protected]> --- .../SqlppExpressionToPlanTranslator.java | 24 ++++++++++----------- .../ASTERIXDB-2750_unnest_join.1.query.sqlpp | 24 +++++++++++++++++++++ .../ASTERIXDB-2750_unnest_join.2.query.sqlpp | 25 ++++++++++++++++++++++ .../ASTERIXDB-2750_unnest_join.1.adm | 1 + .../ASTERIXDB-2750_unnest_join.2.adm | 1 + .../test/resources/runtimets/testsuite_sqlpp.xml | 5 +++++ 6 files changed, 67 insertions(+), 13 deletions(-) diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java index 82dc344..5363ae2 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java @@ -149,7 +149,7 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla public static final String REWRITE_IN_AS_OR_OPTION = "rewrite_in_as_or"; private static final boolean REWRITE_IN_AS_OR_OPTION_DEFAULT = true; - private Deque<Mutable<ILogicalOperator>> uncorrelatedLeftBranchStack = new ArrayDeque<>(); + private Deque<Mutable<ILogicalOperator>> uncorrelatedRightBranchStack = new ArrayDeque<>(); private final Map<VarIdentifier, IAObject> externalVars; private final boolean translateInAsOr; @@ -297,10 +297,12 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla throws CompilationException { Mutable<ILogicalOperator> inputSrc = arg; Pair<ILogicalOperator, LogicalVariable> topUnnest = null; + uncorrelatedRightBranchStack.push(inputSrc); for (FromTerm fromTerm : fromClause.getFromTerms()) { topUnnest = fromTerm.accept(this, inputSrc); inputSrc = new MutableObject<>(topUnnest.first); } + uncorrelatedRightBranchStack.pop(); return topUnnest; } @@ -328,27 +330,20 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla Mutable<ILogicalOperator> topOpRef = new MutableObject<>(unnestOp); if (fromTerm.hasCorrelateClauses()) { for (AbstractBinaryCorrelateClause correlateClause : fromTerm.getCorrelateClauses()) { - if (correlateClause.getClauseType() == ClauseType.UNNEST_CLAUSE) { - // Correlation is allowed. - topOpRef = new MutableObject<>(correlateClause.accept(this, topOpRef).first); - } else { - // Correlation is dis-allowed. - uncorrelatedLeftBranchStack.push(topOpRef); - topOpRef = new MutableObject<>(correlateClause.accept(this, tupSource).first); - } + topOpRef = new MutableObject<>(correlateClause.accept(this, topOpRef).first); } } return new Pair<>(topOpRef.getValue(), fromVar); } @Override - public Pair<ILogicalOperator, LogicalVariable> visit(JoinClause joinClause, Mutable<ILogicalOperator> inputRef) + public Pair<ILogicalOperator, LogicalVariable> visit(JoinClause joinClause, Mutable<ILogicalOperator> leftInputRef) throws CompilationException { SourceLocation sourceLoc = joinClause.getSourceLocation(); - Mutable<ILogicalOperator> leftInputRef = uncorrelatedLeftBranchStack.pop(); if (joinClause.getJoinType() == JoinType.INNER) { + Mutable<ILogicalOperator> rightInputRef = uncorrelatedRightBranchStack.peek(); Pair<ILogicalOperator, LogicalVariable> rightBranch = - generateUnnestForBinaryCorrelateRightBranch(joinClause, inputRef, true); + generateUnnestForBinaryCorrelateRightBranch(joinClause, rightInputRef, true); // A join operator with condition TRUE. AbstractBinaryJoinOperator joinOperator = new InnerJoinOperator( new MutableObject<>(ConstantExpression.TRUE), leftInputRef, new MutableObject<>(rightBranch.first)); @@ -362,7 +357,7 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla filter.getInputs().add(conditionExprOpPair.second); filter.setSourceLocation(conditionExprOpPair.first.getSourceLocation()); return new Pair<>(filter, rightBranch.second); - } else { + } else if (joinClause.getJoinType() == JoinType.LEFTOUTER) { // Creates a subplan operator. SubplanOperator subplanOp = new SubplanOperator(); subplanOp.getInputs().add(leftInputRef); @@ -498,6 +493,9 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla context.setVar(joinClause.getRightVariable(), outerUnnestVar); } return new Pair<>(currentTopOp, null); + } else { + throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, joinClause.getSourceLocation(), + String.valueOf(joinClause.getJoinType().toString())); } } diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.1.query.sqlpp new file mode 100644 index 0000000..c608831 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.1.query.sqlpp @@ -0,0 +1,24 @@ +/* + * 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. + */ + +with + t1 as [ { "x": 1, "y": [10, 11, 12] }, { "x": 2, "y": [20, 21, 22] } ], + t2 as [ 100, 101, 102, 103 ] +from t1 as t1, t1.y as y join t2 as t2 on true +select value count(*) \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.2.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.2.query.sqlpp new file mode 100644 index 0000000..0909785 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.2.query.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. + */ + +with + t1 as [ { "x": 1, "y": [10, 11, 12] }, { "x": 2, "y": [20, 21, 22] } ], + t2 as [ 100, 101, 102, 103 ], + t3 as [ 1000, 1001, 1002, 1003, 1004 ] +from t1 as t1, t1.y as y join t2 as t2 on true join t3 as t3 on true +select value count(*) \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.1.adm new file mode 100644 index 0000000..cabf43b --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.1.adm @@ -0,0 +1 @@ +24 \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.2.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.2.adm new file mode 100644 index 0000000..8bc6583 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/unnest/ASTERIXDB-2750_unnest_join/ASTERIXDB-2750_unnest_join.2.adm @@ -0,0 +1 @@ +120 \ 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 06ccf56..44e2bff 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -12366,6 +12366,11 @@ </test-group> <test-group name="unnest"> <test-case FilePath="unnest"> + <compilation-unit name="ASTERIXDB-2750_unnest_join"> + <output-dir compare="Text">ASTERIXDB-2750_unnest_join</output-dir> + </compilation-unit> + </test-case> + <test-case FilePath="unnest"> <compilation-unit name="left-outer-unnest"> <output-dir compare="Text">left-outer-unnest</output-dir> </compilation-unit>
