This is an automated email from the ASF dual-hosted git repository.
peeyush pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new c51e1e56e0 [ASTERIXDB-3415][COMP] Nested query with union all failing
compilation
c51e1e56e0 is described below
commit c51e1e56e012073f1467a4e2c31aedddf7ee90f7
Author: Peeyush Gupta <[email protected]>
AuthorDate: Thu Jun 20 09:06:58 2024 -0700
[ASTERIXDB-3415][COMP] Nested query with union all failing compilation
- user model changes: no
- storage format changes: no
- interface changes: no
Change-Id: Id4975cc73a51480ef05d6b6d472bf1ca21600000
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18332
Reviewed-by: Peeyush Gupta <[email protected]>
Reviewed-by: Ali Alsuliman <[email protected]>
Tested-by: Peeyush Gupta <[email protected]>
---
.../subplan/InlineAllNtsInSubplanVisitor.java | 22 ++++++-
.../apache/asterix/test/common/TestExecutor.java | 46 +++++++++++++++
.../query-ASTERIXDB-3415.1.ddl.sqlpp | 32 +++++++++++
.../query-ASTERIXDB-3415.2.update.sqlpp | 67 ++++++++++++++++++++++
.../query-ASTERIXDB-3415.3.query.sqlpp | 33 +++++++++++
.../query-ASTERIXDB-3415.4.query.sqlpp | 33 +++++++++++
.../query-ASTERIXDB-3415.3.jsonl | 2 +
.../query-ASTERIXDB-3415.4.jsonl | 2 +
.../src/test/resources/runtimets/sqlpp_queries.xml | 5 ++
.../operators/logical/UnionAllOperator.java | 4 ++
.../rewriter/rules/ExtractCommonOperatorsRule.java | 61 +++++++++++++++++++-
11 files changed, 305 insertions(+), 2 deletions(-)
diff --git
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/subplan/InlineAllNtsInSubplanVisitor.java
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/subplan/InlineAllNtsInSubplanVisitor.java
index 55165ede4a..497d21c236 100644
---
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/subplan/InlineAllNtsInSubplanVisitor.java
+++
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/subplan/InlineAllNtsInSubplanVisitor.java
@@ -592,7 +592,27 @@ class InlineAllNtsInSubplanVisitor implements
IQueryOperatorVisitor<ILogicalOper
@Override
public ILogicalOperator visitUnionOperator(UnionAllOperator op, Void arg)
throws AlgebricksException {
- visitMultiInputOperator(op);
+ orderingExprs.clear();
+ correlatedKeyVars.clear();
+ ILogicalOperator newChild =
op.getInputs().get(0).getValue().accept(this, null);
+ op.getInputs().get(0).setValue(newChild);
+ List<LogicalVariable> leftKeyVars = new ArrayList<>(correlatedKeyVars);
+ correlatedKeyVars.clear();
+
+ newChild = op.getInputs().get(1).getValue().accept(this, null);
+ op.getInputs().get(1).setValue(newChild);
+ int i = 0;
+ for (LogicalVariable leftVar : leftKeyVars) {
+ op.addTriple(new Triple<>(leftKeyVars.get(i),
+
subplanInputVarToCurrentVarMap.get(currentVarToSubplanInputVarMap.get(leftVar)),
context.newVar()));
+ context.computeAndSetTypeEnvironmentForOperator(op);
+ i += 1;
+ }
+ if (correlatedKeyVars.isEmpty()) {
+ correlatedKeyVars.addAll(leftKeyVars);
+ }
+ subtituteVariables(op);
+
// Update the variable mappings
List<Triple<LogicalVariable, LogicalVariable, LogicalVariable>>
varTriples = op.getVariableMappings();
for (Triple<LogicalVariable, LogicalVariable, LogicalVariable> triple
: varTriples) {
diff --git
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
index 20fef82fef..37527bd3a8 100644
---
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
+++
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
@@ -359,6 +359,9 @@ public class TestExecutor {
} else if (actualFile.toString().endsWith(".plan")) {
runScriptAndCompareWithResultPlan(scriptFile, readerExpected,
readerActual);
return;
+ } else if (actualFile.toString().endsWith(".jsonl")) {
+ runScriptAndCompareWithResultJsonl(scriptFile, readerExpected,
readerActual, statement);
+ return;
}
String lineExpected, lineActual;
@@ -636,6 +639,49 @@ public class TestExecutor {
}
}
+ private static void runScriptAndCompareWithResultJsonl(File scriptFile,
BufferedReader readerExpected,
+ BufferedReader readerActual, String statement) throws
ComparisonException, IOException {
+ List<String> expectedLines =
readerExpected.lines().collect(Collectors.toList());
+ List<String> actualLines =
readerActual.lines().collect(Collectors.toList());
+ boolean compareUnorderedArray = statement != null &&
getCompareUnorderedArray(statement);
+ boolean ignoreExtraFields = statement != null &&
getIgnoreExtraFields(statement);
+
+ JsonNode expectedJson, actualJson;
+ int i = 0;
+ for (String expectedLine : expectedLines) {
+ if (actualLines.size() <= i) {
+ throw new ComparisonException("Result for " +
canonicalize(scriptFile) + " expected json line at " + i
+ + " not found: " + truncateIfLong(expectedLine));
+ }
+ String actualLine = actualLines.get(i);
+ i += 1;
+ try {
+ expectedJson = SINGLE_JSON_NODE_READER.readTree(expectedLine);
+ } catch (JsonProcessingException e) {
+ throw new ComparisonException("Invalid expected JSON for: " +
scriptFile, e);
+ }
+ try {
+ actualJson = SINGLE_JSON_NODE_READER.readTree(actualLine);
+ } catch (JsonProcessingException e) {
+ throw new ComparisonException("Invalid actual JSON for: " +
scriptFile, e);
+ }
+ if (expectedJson == null) {
+ throw new ComparisonException("No expected result for: " +
scriptFile);
+ } else if (actualJson == null) {
+ throw new ComparisonException("No actual result for: " +
scriptFile);
+ }
+ if (!TestHelper.equalJson(expectedJson, actualJson,
compareUnorderedArray, ignoreExtraFields, false,
+ null)) {
+ throw new ComparisonException("Result for " + scriptFile + "
didn't match the expected JSON"
+ + "\nexpected result:\n" + expectedJson + "\nactual
result:\n" + actualJson);
+ }
+ }
+ if (actualLines.size() > i) {
+ throw new ComparisonException("Result for " +
canonicalize(scriptFile) + " extra json line at " + i
+ + " found: " + truncateIfLong(actualLines.get(i)));
+ }
+ }
+
public void runScriptAndCompareWithResultUnorderedLinesText(File
scriptFile, BufferedReader readerExpected,
BufferedReader readerActual) throws Exception {
// Using Lists to allow duplicate lines in the result
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.1.ddl.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.1.ddl.sqlpp
new file mode 100644
index 0000000000..774e8a8aef
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.1.ddl.sqlpp
@@ -0,0 +1,32 @@
+/*
+ * 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: This test case is to verify the fix for ASTERIXDB-3415
+ */
+
+drop dataverse test if exists;
+create dataverse test;
+
+use test;
+
+create type dt1 as {id:int};
+create dataset collection1(dt1) primary key id;
+create dataset collection2(dt1) primary key id;
+create dataset collection3(dt1) primary key id;
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.2.update.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.2.update.sqlpp
new file mode 100644
index 0000000000..581408f0da
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.2.update.sqlpp
@@ -0,0 +1,67 @@
+/*
+ * 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 test;
+
+insert into collection1
+([
+ {
+ "id": 1,
+ "f1": "f1"
+ },
+ {
+ "id": 2,
+ "f1": "f1"
+ },
+ {
+ "id": 3,
+ "f1": "f1"
+ }
+]);
+insert into collection2
+([
+ {
+ "id": 1,
+ "f2": "f2"
+ },
+ {
+ "id": 2,
+ "f2": "f2"
+ },
+ {
+ "id": 3,
+ "f2": "f2"
+ }
+]);
+insert into collection3
+([
+ {
+ "id": 1,
+ "f1": "f1",
+ "items": [2, 4]
+
+ },
+ {
+ "id": 2,
+ "f1": "f1",
+ "items": [1]
+ }
+]);
+
+
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.3.query.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.3.query.sqlpp
new file mode 100644
index 0000000000..517000dfc4
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.3.query.sqlpp
@@ -0,0 +1,33 @@
+/*
+ * 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: This test case is to verify the fix for ASTERIXDB-3415
+ */
+-- compareunorderedarray=true
+use test;
+
+SELECT (
+ SELECT c1.f1
+ FROM collection1 AS c1
+ WHERE c1.id IN c3.items
+ UNION ALL
+ SELECT c2.f2
+ FROM collection2 c2 )
+FROM collection3 AS c3
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.4.query.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.4.query.sqlpp
new file mode 100644
index 0000000000..9c1fa2f0d9
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.4.query.sqlpp
@@ -0,0 +1,33 @@
+/*
+ * 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: This test case is to verify the fix for ASTERIXDB-3415
+ */
+-- compareunorderedarray=true
+use test;
+
+SELECT (
+ SELECT c1.f1
+ FROM collection1 AS c1
+ WHERE c1.id = c3.x
+ UNION ALL
+ SELECT c2.f2
+ FROM collection2 c2 )
+FROM collection3 AS c3
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.3.jsonl
b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.3.jsonl
new file mode 100644
index 0000000000..211c0b0340
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.3.jsonl
@@ -0,0 +1,2 @@
+{ "$1": [ { "f2": "f2" }, { "f2": "f2" }, { "f2": "f2" }, { "f1": "f1" } ] }
+{ "$1": [ { "f2": "f2" }, { "f2": "f2" }, { "f2": "f2" }, { "f1": "f1" } ] }
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.4.jsonl
b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.4.jsonl
new file mode 100644
index 0000000000..b28825fadf
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/query-ASTERIXDB-3415/query-ASTERIXDB-3415.4.jsonl
@@ -0,0 +1,2 @@
+{ "$1": [ { "f2": "f2" }, { "f2": "f2" }, { "f2": "f2" } ] }
+{ "$1": [ { "f2": "f2" }, { "f2": "f2" }, { "f2": "f2" } ] }
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
b/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
index 6f5eedd2a6..8abe46c332 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
@@ -7386,6 +7386,11 @@
<output-dir compare="Text">query-ASTERIXDB-3403</output-dir>
</compilation-unit>
</test-case>
+ <test-case FilePath="misc">
+ <compilation-unit name="query-ASTERIXDB-3415">
+ <output-dir compare="Text">query-ASTERIXDB-3415</output-dir>
+ </compilation-unit>
+ </test-case>
</test-group>
<test-group name="multipart-dataverse">
<test-case FilePath="multipart-dataverse">
diff --git
a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/UnionAllOperator.java
b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/UnionAllOperator.java
index 45c6e2f4e5..4d227ea25a 100644
---
a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/UnionAllOperator.java
+++
b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/UnionAllOperator.java
@@ -41,6 +41,10 @@ public class UnionAllOperator extends
AbstractLogicalOperator {
this.varMap = varMap;
}
+ public void addTriple(Triple<LogicalVariable, LogicalVariable,
LogicalVariable> triple) {
+ varMap.add(triple);
+ }
+
public List<Triple<LogicalVariable, LogicalVariable, LogicalVariable>>
getVariableMappings() {
return varMap;
}
diff --git
a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
index fba095aae9..14a44ac6d9 100644
---
a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
+++
b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
@@ -36,12 +36,14 @@ 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.LogicalOperatorTag;
import org.apache.hyracks.algebricks.core.algebra.base.LogicalVariable;
+import org.apache.hyracks.algebricks.core.algebra.base.PhysicalOperatorTag;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractLogicalOperator;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractReplicateOperator;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.AssignOperator;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.ExchangeOperator;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.ProjectOperator;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.ReplicateOperator;
+import
org.apache.hyracks.algebricks.core.algebra.operators.logical.UnionAllOperator;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.IsomorphismUtilities;
import
org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.VariableUtilities;
import
org.apache.hyracks.algebricks.core.algebra.operators.physical.AssignPOperator;
@@ -147,6 +149,63 @@ public class ExtractCommonOperatorsRule implements
IAlgebraicRewriteRule {
return changed;
}
+ // Check for a special case (ASTERIXDB-3415) to avoid adding Replicate
Operator
+ // that causes hang during execution of the plan.
+ private boolean isUnionAllSink(List<Mutable<ILogicalOperator>> group) {
+ if (group.size() != 2) {
+ return false;
+ }
+ List<Pair<Mutable<ILogicalOperator>, Integer>> operators = new
ArrayList<>();
+ if (childrenToParents.containsKey(group.get(0))) {
+ for (Mutable<ILogicalOperator> op :
childrenToParents.get(group.get(0))) {
+ operators.add(new Pair<>(op, 0));
+ }
+ } else {
+ return false;
+ }
+ List<Pair<Mutable<ILogicalOperator>, Integer>> unionAllOps = new
ArrayList<>();
+ while (operators.size() > 0) {
+ Pair<Mutable<ILogicalOperator>, Integer> entry =
operators.remove(0);
+ if (entry.first.getValue() instanceof UnionAllOperator) {
+ unionAllOps.add(entry);
+ } else if (entry.first.getValue() instanceof ExchangeOperator &&
((ExchangeOperator) entry.first.getValue())
+ .getPhysicalOperator().getOperatorTag() ==
PhysicalOperatorTag.HASH_PARTITION_EXCHANGE) {
+ entry.second += 1;
+ }
+ if (childrenToParents.containsKey(entry.first)) {
+ for (Mutable<ILogicalOperator> op :
childrenToParents.get(entry.first)) {
+ operators.add(new Pair<>(op, entry.second));
+ }
+ }
+ }
+ if (childrenToParents.containsKey(group.get(1))) {
+ for (Mutable<ILogicalOperator> op :
childrenToParents.get(group.get(1))) {
+ operators.add(new Pair<>(op, 0));
+ }
+ } else {
+ return false;
+ }
+ while (operators.size() > 0) {
+ Pair<Mutable<ILogicalOperator>, Integer> entry =
operators.remove(0);
+ if (entry.first.getValue() instanceof UnionAllOperator) {
+ for (Pair<Mutable<ILogicalOperator>, Integer> unionAllOp :
unionAllOps) {
+ if (unionAllOp.first.equals(entry.first) &&
unionAllOp.second + entry.second == 1) {
+ return true;
+ }
+ }
+ } else if (entry.first.getValue() instanceof ExchangeOperator &&
((ExchangeOperator) entry.first.getValue())
+ .getPhysicalOperator().getOperatorTag() ==
PhysicalOperatorTag.HASH_PARTITION_EXCHANGE) {
+ entry.second += 1;
+ }
+ if (childrenToParents.containsKey(entry.first)) {
+ for (Mutable<ILogicalOperator> op :
childrenToParents.get(entry.first)) {
+ operators.add(new Pair<>(op, entry.second));
+ }
+ }
+ }
+ return false;
+ }
+
private boolean
rewriteForOneEquivalentClass(List<Mutable<ILogicalOperator>> members,
IOptimizationContext context)
throws AlgebricksException {
List<Mutable<ILogicalOperator>> group = new ArrayList<>();
@@ -163,7 +222,7 @@ public class ExtractCommonOperatorsRule implements
IAlgebraicRewriteRule {
}
}
boolean[] materializationFlags = computeMaterilizationFlags(group);
- if (group.isEmpty()) {
+ if (group.isEmpty() || isUnionAllSink(group)) {
continue;
}
candidate = group.get(0);
