You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by dl...@apache.org on 2019/07/18 17:22:34 UTC

[asterixdb] branch master updated: [NO ISSUE][COMP] Improve IPartitioningProperty.substituteColumnVars()

This is an automated email from the ASF dual-hosted git repository.

dlych 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 3c71d2b  [NO ISSUE][COMP] Improve IPartitioningProperty.substituteColumnVars()
3c71d2b is described below

commit 3c71d2bd02a68cf3eedfab1491d7bbe6911649bc
Author: Dmitry Lychagin <dm...@couchbase.com>
AuthorDate: Wed Jul 17 14:19:19 2019 -0700

    [NO ISSUE][COMP] Improve IPartitioningProperty.substituteColumnVars()
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    - IPartitioningProperty.substituteColumnVars() should return
      a new instance of IPartitioningProperty if variable
      substitution was performed for given variables
    - Compute schema for a new Select operator introduced by
      PullSelectOutOfEqJoin rule
    - Refactor delivered properties computation in
      AbstractPreclusteredGroupByPOperator
    
    Change-Id: Iee7ef7de26f9d960b205d6d43e2820aaa396cb15
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/3499
    Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
 .../AbstractPreclusteredGroupByPOperator.java      | 50 +++++++++++-----------
 .../operators/physical/IntersectPOperator.java     |  4 +-
 .../properties/BroadcastPartitioningProperty.java  |  8 +++-
 .../algebra/properties/IPartitioningProperty.java  |  7 +--
 .../properties/OrderedPartitionedProperty.java     | 17 +++++---
 .../properties/RandomPartitioningProperty.java     |  4 +-
 .../properties/UnorderedPartitionedProperty.java   | 16 ++++---
 .../rewriter/rules/PullSelectOutOfEqJoin.java      |  1 +
 8 files changed, 63 insertions(+), 44 deletions(-)

diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractPreclusteredGroupByPOperator.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractPreclusteredGroupByPOperator.java
index fdb7347..70f7e2b 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractPreclusteredGroupByPOperator.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractPreclusteredGroupByPOperator.java
@@ -63,35 +63,37 @@ public abstract class AbstractPreclusteredGroupByPOperator extends AbstractGroup
         super(columnList);
     }
 
-    // Obs: We don't propagate properties corresponding to decors, since they
-    // are func. dep. on the group-by variables.
+    // Obs: We don't propagate properties corresponding to decors, since they are func. dep. on the group-by variables.
     @Override
     public void computeDeliveredProperties(ILogicalOperator op, IOptimizationContext context) {
-        List<ILocalStructuralProperty> propsLocal = new ArrayList<>();
         GroupByOperator gby = (GroupByOperator) op;
-        ILogicalOperator op2 = gby.getInputs().get(0).getValue();
-        IPhysicalPropertiesVector childProp = op2.getDeliveredPhysicalProperties();
-        IPartitioningProperty pp = childProp.getPartitioningProperty();
-        Map<LogicalVariable, LogicalVariable> ppSubstMap = computePartitioningPropertySubstitutionMap(gby, pp);
-        if (ppSubstMap != null) {
-            // We cannot modify pp directly, since it is owned by the input operator.
-            // Otherwise, the partitioning property would be modified even before this group by operator,
-            // which will be undesirable.
-            pp = pp.clonePartitioningProperty();
-            pp.substituteColumnVars(ppSubstMap);
-        }
-        List<ILocalStructuralProperty> childLocals = childProp.getLocalProperties();
-        if (childLocals == null) {
-            deliveredProperties = new StructuralPropertiesVector(pp, propsLocal);
-            return;
-        }
-        for (ILocalStructuralProperty lsp : childLocals) {
-            ILocalStructuralProperty propagatedLsp = getPropagatedProperty(lsp, gby);
-            if (propagatedLsp != null) {
-                propsLocal.add(propagatedLsp);
+        ILogicalOperator childOp = gby.getInputs().get(0).getValue();
+        IPhysicalPropertiesVector childProperties = childOp.getDeliveredPhysicalProperties();
+        IPartitioningProperty partitioning =
+                computePartitioningProperty(gby, childProperties.getPartitioningProperty());
+        List<ILocalStructuralProperty> local = computeLocalProperties(gby, childProperties.getLocalProperties());
+        deliveredProperties = new StructuralPropertiesVector(partitioning, local);
+    }
+
+    private IPartitioningProperty computePartitioningProperty(GroupByOperator gby,
+            IPartitioningProperty childPartitioning) {
+        Map<LogicalVariable, LogicalVariable> substMap =
+                computePartitioningPropertySubstitutionMap(gby, childPartitioning);
+        return substMap != null ? childPartitioning.substituteColumnVars(substMap) : childPartitioning;
+    }
+
+    private List<ILocalStructuralProperty> computeLocalProperties(GroupByOperator gby,
+            List<ILocalStructuralProperty> childLocals) {
+        List<ILocalStructuralProperty> propsLocal = new ArrayList<>();
+        if (childLocals != null) {
+            for (ILocalStructuralProperty lsp : childLocals) {
+                ILocalStructuralProperty propagatedLsp = getPropagatedProperty(lsp, gby);
+                if (propagatedLsp != null) {
+                    propsLocal.add(propagatedLsp);
+                }
             }
         }
-        deliveredProperties = new StructuralPropertiesVector(pp, propsLocal);
+        return propsLocal;
     }
 
     // If we have "gby var1 as var3, var2 as var4"
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/IntersectPOperator.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/IntersectPOperator.java
index 3cf0fb9..9a595c5 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/IntersectPOperator.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/IntersectPOperator.java
@@ -89,7 +89,7 @@ public class IntersectPOperator extends AbstractPhysicalOperator {
     @Override
     public void computeDeliveredProperties(ILogicalOperator iop, IOptimizationContext context) {
         IntersectOperator op = (IntersectOperator) iop;
-        IPartitioningProperty pp =
+        IPartitioningProperty childpp =
                 op.getInputs().get(0).getValue().getDeliveredPhysicalProperties().getPartitioningProperty();
 
         List<LogicalVariable> outputCompareVars = op.getOutputCompareVariables();
@@ -98,7 +98,7 @@ public class IntersectPOperator extends AbstractPhysicalOperator {
         for (int i = 0; i < numCompareVars; i++) {
             varMaps.put(op.getInputCompareVariables(0).get(i), outputCompareVars.get(i));
         }
-        pp.substituteColumnVars(varMaps);
+        IPartitioningProperty pp = childpp.substituteColumnVars(varMaps);
 
         List<ILocalStructuralProperty> propsLocal = new ArrayList<>();
         List<OrderColumn> orderColumns = new ArrayList<>();
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/BroadcastPartitioningProperty.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/BroadcastPartitioningProperty.java
index 3e78fd2..3a7f742 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/BroadcastPartitioningProperty.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/BroadcastPartitioningProperty.java
@@ -59,8 +59,8 @@ public class BroadcastPartitioningProperty implements IPartitioningProperty {
     }
 
     @Override
-    public void substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
-
+    public IPartitioningProperty substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
+        return this;
     }
 
     @Override
@@ -68,4 +68,8 @@ public class BroadcastPartitioningProperty implements IPartitioningProperty {
         return new BroadcastPartitioningProperty(domain);
     }
 
+    @Override
+    public String toString() {
+        return getPartitioningType().toString() + " domain:" + domain;
+    }
 }
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningProperty.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningProperty.java
index 5164192..f5eef1c 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningProperty.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningProperty.java
@@ -76,7 +76,7 @@ public interface IPartitioningProperty extends IStructuralProperty {
 
     void setNodeDomain(INodeDomain domain);
 
-    void substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap);
+    IPartitioningProperty substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap);
 
     IPartitioningProperty clonePartitioningProperty();
 }
@@ -115,12 +115,13 @@ class UnpartitionedProperty implements IPartitioningProperty {
     }
 
     @Override
-    public void substituteColumnVars(Map<LogicalVariable, LogicalVariable> variableMap) {
+    public IPartitioningProperty substituteColumnVars(Map<LogicalVariable, LogicalVariable> variableMap) {
         // No partition columns are maintained for UNPARTITIONED.
+        return UNPARTITIONED;
     }
 
     @Override
     public IPartitioningProperty clonePartitioningProperty() {
-        return new UnpartitionedProperty();
+        return UNPARTITIONED;
     }
 }
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/OrderedPartitionedProperty.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/OrderedPartitionedProperty.java
index b5a2bb5..7112ef7 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/OrderedPartitionedProperty.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/OrderedPartitionedProperty.java
@@ -55,7 +55,7 @@ public class OrderedPartitionedProperty implements IPartitioningProperty {
 
     @Override
     public String toString() {
-        return getPartitioningType().toString() + orderColumns;
+        return getPartitioningType().toString() + orderColumns + " domain:" + domain;
     }
 
     @Override
@@ -84,17 +84,24 @@ public class OrderedPartitionedProperty implements IPartitioningProperty {
     }
 
     @Override
-    public void substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
+    public IPartitioningProperty substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
+        boolean applied = false;
+        List<OrderColumn> newOrderColumns = new ArrayList<>(orderColumns.size());
         for (OrderColumn orderColumn : orderColumns) {
-            if (varMap.containsKey(orderColumn.getColumn())) {
-                orderColumn.setColumn(varMap.get(orderColumn.getColumn()));
+            LogicalVariable columnVar = orderColumn.getColumn();
+            LogicalVariable newColumnVar = varMap.get(columnVar);
+            if (newColumnVar != null) {
+                applied = true;
+            } else {
+                newColumnVar = columnVar;
             }
+            newOrderColumns.add(new OrderColumn(newColumnVar, orderColumn.getOrder()));
         }
+        return applied ? new OrderedPartitionedProperty(newOrderColumns, domain) : this;
     }
 
     @Override
     public IPartitioningProperty clonePartitioningProperty() {
         return new OrderedPartitionedProperty(new ArrayList<>(orderColumns), domain);
     }
-
 }
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/RandomPartitioningProperty.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/RandomPartitioningProperty.java
index 951a031..f5c7aa8 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/RandomPartitioningProperty.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/RandomPartitioningProperty.java
@@ -64,8 +64,8 @@ public class RandomPartitioningProperty implements IPartitioningProperty {
     }
 
     @Override
-    public void substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
-
+    public IPartitioningProperty substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
+        return this;
     }
 
     @Override
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/UnorderedPartitionedProperty.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/UnorderedPartitionedProperty.java
index 5966407..fa8650c 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/UnorderedPartitionedProperty.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/UnorderedPartitionedProperty.java
@@ -51,7 +51,7 @@ public final class UnorderedPartitionedProperty extends AbstractGroupingProperty
 
     @Override
     public String toString() {
-        return getPartitioningType().toString() + columnSet;
+        return getPartitioningType().toString() + columnSet + " domain:" + domain;
     }
 
     @Override
@@ -70,12 +70,16 @@ public final class UnorderedPartitionedProperty extends AbstractGroupingProperty
     }
 
     @Override
-    public void substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
-        varMap.forEach((key, value) -> {
-            if (columnSet.remove(key)) {
-                columnSet.add(value);
+    public IPartitioningProperty substituteColumnVars(Map<LogicalVariable, LogicalVariable> varMap) {
+        boolean applied = false;
+        Set<LogicalVariable> newColumnSet = new ListSet<>(columnSet);
+        for (Map.Entry<LogicalVariable, LogicalVariable> me : varMap.entrySet()) {
+            if (newColumnSet.remove(me.getKey())) {
+                newColumnSet.add(me.getValue());
+                applied = true;
             }
-        });
+        }
+        return applied ? new UnorderedPartitionedProperty(newColumnSet, domain) : this;
     }
 
     @Override
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PullSelectOutOfEqJoin.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PullSelectOutOfEqJoin.java
index 6ba6a0f..afbbc4f 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PullSelectOutOfEqJoin.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PullSelectOutOfEqJoin.java
@@ -84,6 +84,7 @@ public class PullSelectOutOfEqJoin implements IAlgebraicRewriteRule {
         ILogicalExpression newJoinCond = makeCondition(eqVarVarComps, context);
         join.getCondition().setValue(newJoinCond);
         select.getInputs().add(new MutableObject<ILogicalOperator>(join));
+        select.recomputeSchema();
         opRef.setValue(select);
         context.computeAndSetTypeEnvironmentForOperator(select);
         return true;