You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2022/06/01 16:55:48 UTC

Change in asterixdb[master]: [ASTERIXDB-3038][COMP] Check partitioning in ExtractCommonOperatorsRule

From Dmitry Lychagin <dm...@couchbase.com>:

Dmitry Lychagin has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16423 )


Change subject: [ASTERIXDB-3038][COMP] Check partitioning in ExtractCommonOperatorsRule
......................................................................

[ASTERIXDB-3038][COMP] Check partitioning in ExtractCommonOperatorsRule

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- ExtractCommonOperatorsRule should not introduce
  replicate operator if candidate plan segments
  have different partitiong properties
- Set UnnestMap execution mode to PARTITIONED
  when introducing index-search

Change-Id: I79ae17331287aa482a35b521c505d0f201b86063
---
M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.1.ddl.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.2.query.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/results/misc/metadata_only_02/metadata_only_02.2.adm
M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
M hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
6 files changed, 124 insertions(+), 12 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/23/16423/1

diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
index 5dd0135..2eebe5b 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
@@ -753,7 +753,7 @@
             } else {
                 leftOuterUnnestMapRequired = false;
             }
-
+            AbstractUnnestMapOperator unnestMapOp;
             if (conditionRef.getValue() != null) {
                 // The job gen parameters are transferred to the actual job gen
                 // via the UnnestMapOperator's function arguments.
@@ -765,7 +765,6 @@
                         new UnnestingFunctionCallExpression(primaryIndexSearch, primaryIndexFuncArgs);
                 primaryIndexSearchFunc.setSourceLocation(dataSourceOp.getSourceLocation());
                 primaryIndexSearchFunc.setReturnsUniqueValues(true);
-                AbstractUnnestMapOperator unnestMapOp;
                 if (!leftOuterUnnestMapRequired) {
                     unnestMapOp = new UnnestMapOperator(scanVariables,
                             new MutableObject<ILogicalExpression>(primaryIndexSearchFunc), primaryIndexOutputTypes,
@@ -775,10 +774,7 @@
                             new MutableObject<ILogicalExpression>(primaryIndexSearchFunc), primaryIndexOutputTypes,
                             leftOuterMissingValue);
                 }
-                unnestMapOp.setSourceLocation(dataSourceOp.getSourceLocation());
-                indexSearchOp = unnestMapOp;
             } else {
-                AbstractUnnestMapOperator unnestMapOp;
                 if (!leftOuterUnnestMapRequired) {
                     unnestMapOp = new UnnestMapOperator(scanVariables,
                             ((UnnestMapOperator) secondaryIndexUnnestOp).getExpressionRef(), primaryIndexOutputTypes,
@@ -788,11 +784,11 @@
                             ((LeftOuterUnnestMapOperator) secondaryIndexUnnestOp).getExpressionRef(),
                             primaryIndexOutputTypes, leftOuterMissingValue);
                 }
-                unnestMapOp.setSourceLocation(dataSourceOp.getSourceLocation());
-                indexSearchOp = unnestMapOp;
             }
-            // TODO: shouldn't indexSearchOp execution mode be set to that of the input? the default is UNPARTITIONED
-            indexSearchOp.getInputs().add(new MutableObject<>(inputOp));
+            unnestMapOp.setExecutionMode(ExecutionMode.PARTITIONED);
+            unnestMapOp.setSourceLocation(dataSourceOp.getSourceLocation());
+            unnestMapOp.getInputs().add(new MutableObject<>(inputOp));
+            indexSearchOp = unnestMapOp;
 
             // Adds equivalence classes --- one equivalent class between a primary key
             // variable and a record field-access expression.
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.1.ddl.sqlpp
new file mode 100644
index 0000000..ab247a3
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.1.ddl.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * 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.
+ */
+
+/*
+ * The query tests fix for ASTERIXDB-3038
+ */
+
+drop dataverse test1 if exists;
+drop dataverse test2 if exists;
+
+create dataverse test1;
+create dataverse test2;
+
+create dataset test1.ds1 (id1 integer not unknown) primary key id1;
+create dataset test2.ds2 (id2 integer not unknown) primary key id2;
+
+create synonym test1.syn1 for test1.ds1;
+create synonym test2.syn2 for test2.ds2;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.2.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.2.query.sqlpp
new file mode 100644
index 0000000..a7e1b93
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/metadata_only_02/metadata_only_02.2.query.sqlpp
@@ -0,0 +1,49 @@
+/*
+ * 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.
+ */
+
+/*
+ * The query tests fix for ASTERIXDB-3038
+ */
+
+select syn.DataverseName, syn.SynonymName
+from Metadata.`Synonym` as syn
+where syn.ObjectDataverseName in
+  ["test1", "test2"]
+and syn.ObjectName in (
+  select value ds.DatasetName
+  from Metadata.`Dataset` as ds
+  where ds.DataverseName in
+    ["test1", "test2"]
+  and ds.DatasetName in
+    ["ds1", "ds2"])
+  or syn.ObjectName in (
+    select value syn1.SynonymName
+    from Metadata.`Synonym` as syn1
+    where syn1.SynonymName in
+      ["syn1", "syn2"]
+    and syn1.ObjectName in (
+      select value ds1.DatasetName
+      from Metadata.`Dataset` as ds1
+      where ds1.DataverseName in
+        ["test1", "test2"]
+      and ds1.DatasetName in
+        ["ds1", "ds2"]
+    )
+  )
+order by syn.DataverseName, syn.SynonymName;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/metadata_only_02/metadata_only_02.2.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/metadata_only_02/metadata_only_02.2.adm
new file mode 100644
index 0000000..743bb8a
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/metadata_only_02/metadata_only_02.2.adm
@@ -0,0 +1,2 @@
+{ "DataverseName": "test1", "SynonymName": "syn1" }
+{ "DataverseName": "test2", "SynonymName": "syn2" }
\ 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 1267995..5d5980f 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -7207,6 +7207,11 @@
       </compilation-unit>
     </test-case>
     <test-case FilePath="misc">
+      <compilation-unit name="metadata_only_02">
+        <output-dir compare="Text">metadata_only_02</output-dir>
+      </compilation-unit>
+    </test-case>
+    <test-case FilePath="misc">
       <compilation-unit name="cast-ASTERIXDB-2458">
         <output-dir compare="Text">cast-ASTERIXDB-2458</output-dir>
       </compilation-unit>
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 bd24644..10a8da7 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
@@ -48,6 +48,7 @@
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.OneToOneExchangePOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.ReplicatePOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.StreamProjectPOperator;
+import org.apache.hyracks.algebricks.core.algebra.properties.IPartitioningProperty;
 import org.apache.hyracks.algebricks.core.algebra.util.OperatorManipulationUtil;
 import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
 import org.apache.hyracks.api.exceptions.SourceLocation;
@@ -88,6 +89,10 @@
     public boolean rewritePost(Mutable<ILogicalOperator> opRef, IOptimizationContext context)
             throws AlgebricksException {
         AbstractLogicalOperator op = (AbstractLogicalOperator) opRef.getValue();
+        // compute delivered properties bottom up if necessary
+        if (op.getDeliveredPhysicalProperties() == null) {
+            op.computeDeliveredPhysicalProperties(context);
+        }
         if (op.getOperatorTag() != LogicalOperatorTag.WRITE && op.getOperatorTag() != LogicalOperatorTag.WRITE_RESULT
                 && op.getOperatorTag() != LogicalOperatorTag.DISTRIBUTE_RESULT) {
             return false;
@@ -187,10 +192,12 @@
                 beforeExchange.getInputs().add(candidate);
                 context.computeAndSetTypeEnvironmentForOperator(beforeExchange);
                 beforeExchange.recomputeSchema();
+                beforeExchange.computeDeliveredPhysicalProperties(context);
                 rop.getInputs().add(beforeExchangeRef);
             }
             context.computeAndSetTypeEnvironmentForOperator(rop);
             rop.recomputeSchema();
+            rop.computeDeliveredPhysicalProperties(context);
 
             for (Mutable<ILogicalOperator> parentRef : originalCandidateParents) {
                 AbstractLogicalOperator parent = (AbstractLogicalOperator) parentRef.getValue();
@@ -207,9 +214,11 @@
                     rop.getOutputs().add(exchangeRef);
                     context.computeAndSetTypeEnvironmentForOperator(exchange);
                     exchange.recomputeSchema();
+                    exchange.computeDeliveredPhysicalProperties(context);
                     parent.getInputs().set(index, exchangeRef);
                     context.computeAndSetTypeEnvironmentForOperator(parent);
                     parent.recomputeSchema();
+                    parent.computeDeliveredPhysicalProperties(context);
                 }
             }
             List<LogicalVariable> liveVarsNew = new ArrayList<>();
@@ -254,10 +263,13 @@
                 // set the types
                 context.computeAndSetTypeEnvironmentForOperator(exchOp);
                 exchOp.recomputeSchema();
+                exchOp.computeDeliveredPhysicalProperties(context);
                 context.computeAndSetTypeEnvironmentForOperator(assignOperator);
                 assignOperator.recomputeSchema();
+                assignOperator.computeDeliveredPhysicalProperties(context);
                 context.computeAndSetTypeEnvironmentForOperator(projectOperator);
                 projectOperator.recomputeSchema();
+                projectOperator.computeDeliveredPhysicalProperties(context);
 
                 List<Mutable<ILogicalOperator>> parentOpList = childrenToParents.get(ref);
                 for (Mutable<ILogicalOperator> parentOpRef : parentOpList) {
@@ -278,9 +290,11 @@
                         parentOp.getInputs().set(index, new MutableObject<ILogicalOperator>(exchg));
                         context.computeAndSetTypeEnvironmentForOperator(exchg);
                         exchg.recomputeSchema();
+                        exchg.computeDeliveredPhysicalProperties(context);
                     }
                     context.computeAndSetTypeEnvironmentForOperator(parentOp);
                     parentOp.recomputeSchema();
+                    parentOp.computeDeliveredPhysicalProperties(context);
                 }
             }
             cleanupPlan();
@@ -471,9 +485,8 @@
                     equivalentClass.add(candidates.get(i));
                     for (int j = i - 1; j >= 0; j--) {
                         ILogicalOperator peer = candidates.get(j).getValue();
-                        boolean isomorphic = candidate.getInputs().size() > 1
-                                ? IsomorphismUtilities.isOperatorIsomorphicPlanSegment(candidate, peer)
-                                : IsomorphismUtilities.isOperatorIsomorphic(candidate, peer);
+                        boolean isomorphic = isIsomorphicOperator(candidate, peer)
+                                && isIsomorphicPartitioningProperty(candidate, peer);
                         if (isomorphic) {
                             reserved[i] = true;
                             reserved[j] = true;
@@ -494,6 +507,19 @@
         }
     }
 
+    private static boolean isIsomorphicOperator(ILogicalOperator candidate, ILogicalOperator peer)
+            throws AlgebricksException {
+        return candidate.getInputs().size() > 1 ? IsomorphismUtilities.isOperatorIsomorphicPlanSegment(candidate, peer)
+                : IsomorphismUtilities.isOperatorIsomorphic(candidate, peer);
+    }
+
+    private static boolean isIsomorphicPartitioningProperty(ILogicalOperator candidate, ILogicalOperator peer) {
+        IPartitioningProperty ppCandidate = candidate.getDeliveredPhysicalProperties().getPartitioningProperty();
+        IPartitioningProperty ppPeer = peer.getDeliveredPhysicalProperties().getPartitioningProperty();
+        return (ppCandidate.getPartitioningType() == ppPeer.getPartitioningType())
+                && ppCandidate.getNodeDomain().sameAs(ppPeer.getNodeDomain());
+    }
+
     private boolean[] computeMaterilizationFlags(List<Mutable<ILogicalOperator>> group) {
         for (Mutable<ILogicalOperator> root : roots) {
             computeClusters(null, root, new MutableInt(++lastUsedClusterId));

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/16423
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I79ae17331287aa482a35b521c505d0f201b86063
Gerrit-Change-Number: 16423
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-MessageType: newchange