Luo Chen has submitted this change and it was merged.

Change subject: [ASTERIXDB-2119][COMP] Fix variable ordering of project pushdown
......................................................................


[ASTERIXDB-2119][COMP] Fix variable ordering of project pushdown

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

Details:

Fix variable ordering of PushProjectDownRule by using LinkedHashSet.
Related to previous patch https://asterix-gerrit.ics.uci.edu/#/c/2043/

Change-Id: I21d0a2764e22482a9361b709e566d78acd33cf4d
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2048
Tested-by: Jenkins <[email protected]>
Contrib: Jenkins <[email protected]>
Integration-Tests: Jenkins <[email protected]>
Reviewed-by: Jianfeng Jia <[email protected]>
---
M 
asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access/explain_field_access.1.adm
M 
asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access_closed/explain_field_access_closed.1.adm
M 
asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.13.adm
M 
asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.8.adm
M 
hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java
5 files changed, 16 insertions(+), 15 deletions(-)

Approvals:
  Jenkins: Verified; ; Verified
  Jianfeng Jia: Looks good to me, approved

Objections:
  Anon. E. Moose #1000171: 
  Jenkins: Violations found



diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access/explain_field_access.1.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access/explain_field_access.1.adm
index daa29c4..5f6b71d 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access/explain_field_access.1.adm
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access/explain_field_access.1.adm
@@ -26,7 +26,7 @@
               -- SORT_GROUP_BY[$$18]  |PARTITIONED|
                 exchange
                 -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
-                  project ([$$18, $$15])
+                  project ([$$15, $$18])
                   -- STREAM_PROJECT  |PARTITIONED|
                     assign [$$18] <- 
[substring($$22.getField("department_id"), 1)]
                     -- ASSIGN  |PARTITIONED|
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access_closed/explain_field_access_closed.1.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access_closed/explain_field_access_closed.1.adm
index 864391b..c984482 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access_closed/explain_field_access_closed.1.adm
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/explain/explain_field_access_closed/explain_field_access_closed.1.adm
@@ -26,7 +26,7 @@
               -- SORT_GROUP_BY[$$17]  |PARTITIONED|
                 exchange
                 -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
-                  project ([$$17, $$14])
+                  project ([$$14, $$17])
                   -- STREAM_PROJECT  |PARTITIONED|
                     assign [$$17, $$14] <- [substring($$e.getField(1), 1), 
$$e.getField(2)]
                     -- ASSIGN  |PARTITIONED|
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.13.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.13.adm
index c7b09c6..a76e980 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.13.adm
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.13.adm
@@ -18,7 +18,7 @@
                 -- STABLE_SORT [$$15(ASC), $$16(ASC)]  |PARTITIONED|
                   exchange
                   -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
-                    project ([$$16, $$15])
+                    project ([$$15, $$16])
                     -- STREAM_PROJECT  |PARTITIONED|
                       exchange
                       -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.8.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.8.adm
index c7b09c6..a76e980 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.8.adm
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/rebalance/single_dataset_with_index/single_dataset_with_index.8.adm
@@ -18,7 +18,7 @@
                 -- STABLE_SORT [$$15(ASC), $$16(ASC)]  |PARTITIONED|
                   exchange
                   -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
-                    project ([$$16, $$15])
+                    project ([$$15, $$16])
                     -- STREAM_PROJECT  |PARTITIONED|
                       exchange
                       -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
diff --git 
a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java
 
b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java
index 88c0ea9..5675a03 100644
--- 
a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java
+++ 
b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java
@@ -20,9 +20,10 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.commons.lang3.mutable.Mutable;
 import org.apache.commons.lang3.mutable.MutableObject;
@@ -64,7 +65,7 @@
         ProjectOperator pi = (ProjectOperator) op;
         Mutable<ILogicalOperator> opRef2 = pi.getInputs().get(0);
 
-        HashSet<LogicalVariable> toPush = new HashSet<LogicalVariable>();
+        Set<LogicalVariable> toPush = new LinkedHashSet<LogicalVariable>();
         toPush.addAll(pi.getVariables());
 
         Pair<Boolean, Boolean> p = pushThroughOp(toPush, opRef2, op, context);
@@ -77,9 +78,8 @@
         return smthWasPushed;
     }
 
-    private static Pair<Boolean, Boolean> 
pushThroughOp(HashSet<LogicalVariable> toPush,
-            Mutable<ILogicalOperator> opRef2, ILogicalOperator initialOp, 
IOptimizationContext context)
-                    throws AlgebricksException {
+    private static Pair<Boolean, Boolean> pushThroughOp(Set<LogicalVariable> 
toPush, Mutable<ILogicalOperator> opRef2,
+            ILogicalOperator initialOp, IOptimizationContext context) throws 
AlgebricksException {
         List<LogicalVariable> initProjectList = new 
ArrayList<LogicalVariable>(toPush);
         AbstractLogicalOperator op2 = (AbstractLogicalOperator) 
opRef2.getValue();
         do {
@@ -112,13 +112,14 @@
 
         boolean canCommuteProjection = initProjectList.containsAll(toPush) && 
initProjectList.containsAll(produced2)
                 && initProjectList.containsAll(used2);
-                // if true, we can get rid of the initial projection
+        // if true, we can get rid of the initial projection
 
         // get rid of useless decor vars.
         if (!canCommuteProjection && op2.getOperatorTag() == 
LogicalOperatorTag.GROUP) {
             boolean gbyChanged = false;
             GroupByOperator gby = (GroupByOperator) op2;
-            List<Pair<LogicalVariable, Mutable<ILogicalExpression>>> 
newDecorList = new ArrayList<Pair<LogicalVariable, 
Mutable<ILogicalExpression>>>();
+            List<Pair<LogicalVariable, Mutable<ILogicalExpression>>> 
newDecorList =
+                    new ArrayList<Pair<LogicalVariable, 
Mutable<ILogicalExpression>>>();
             for (Pair<LogicalVariable, Mutable<ILogicalExpression>> p : 
gby.getDecorList()) {
                 LogicalVariable decorVar = GroupByOperator.getDecorVariable(p);
                 if (!toPush.contains(decorVar)) {
@@ -164,13 +165,13 @@
     }
 
     // It does not try to push above another Projection.
-    private static boolean pushNeededProjections(HashSet<LogicalVariable> 
toPush, Mutable<ILogicalOperator> opRef,
+    private static boolean pushNeededProjections(Set<LogicalVariable> toPush, 
Mutable<ILogicalOperator> opRef,
             IOptimizationContext context, ILogicalOperator initialOp) throws 
AlgebricksException {
-        HashSet<LogicalVariable> allP = new HashSet<LogicalVariable>();
+        Set<LogicalVariable> allP = new LinkedHashSet<LogicalVariable>();
         AbstractLogicalOperator op = (AbstractLogicalOperator) 
opRef.getValue();
         VariableUtilities.getSubplanLocalLiveVariables(op, allP);
 
-        HashSet<LogicalVariable> toProject = new HashSet<LogicalVariable>();
+        Set<LogicalVariable> toProject = new LinkedHashSet<LogicalVariable>();
         for (LogicalVariable v : toPush) {
             if (allP.contains(v)) {
                 toProject.add(v);
@@ -192,7 +193,7 @@
     // It does not try to push above another Projection.
     private static boolean 
pushAllProjectionsOnTopOf(Collection<LogicalVariable> toPush,
             Mutable<ILogicalOperator> opRef, IOptimizationContext context, 
ILogicalOperator initialOp)
-                    throws AlgebricksException {
+            throws AlgebricksException {
         if (toPush.isEmpty()) {
             return false;
         }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2048
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I21d0a2764e22482a9361b709e566d78acd33cf4d
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Jianfeng Jia <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>

Reply via email to