>From Ali Alsuliman <[email protected]>: Ali Alsuliman has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19531 )
Change subject: [ASTERIXDB-3580][COMP] minor refactoring ...................................................................... [ASTERIXDB-3580][COMP] minor refactoring Ext-ref: MB-63354, MB-65314 Change-Id: I270201573d5d19763bf8d68db99faa72f0b92f46 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19531 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> Reviewed-by: Peeyush Gupta <[email protected]> --- M hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractHashJoinPOperator.java M hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningRequirementsCoordinator.java 2 files changed, 76 insertions(+), 48 deletions(-) Approvals: Ali Alsuliman: Looks good to me, but someone else must approve Peeyush Gupta: Looks good to me, approved Jenkins: Verified; Verified Anon. E. Moose #1000171: diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractHashJoinPOperator.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractHashJoinPOperator.java index 68f0898..800c839 100644 --- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractHashJoinPOperator.java +++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/AbstractHashJoinPOperator.java @@ -138,19 +138,21 @@ .getPartitioningType() == requirements.getPartitioningType()) { switch (requirements.getPartitioningType()) { case UNORDERED_PARTITIONED: { - UnorderedPartitionedProperty upp1 = + UnorderedPartitionedProperty unorderedFirstDelivered = (UnorderedPartitionedProperty) firstDeliveredPartitioning; - Set<LogicalVariable> set1 = upp1.getColumnSet(); - UnorderedPartitionedProperty uppreq = (UnorderedPartitionedProperty) requirements; - Set<LogicalVariable> modifuppreq = new ListSet<>(); + Set<LogicalVariable> firstDeliveredVars = unorderedFirstDelivered.getColumnSet(); + UnorderedPartitionedProperty unorderedRequired = + (UnorderedPartitionedProperty) requirements; + Set<LogicalVariable> originalRequiredVars = unorderedRequired.getColumnSet(); + Set<LogicalVariable> modifiedRequiredVars = new ListSet<>(); Map<LogicalVariable, EquivalenceClass> eqmap = context.getEquivalenceClassMap(op); - Set<LogicalVariable> covered = new ListSet<>(); - Set<LogicalVariable> keysCurrent = uppreq.getColumnSet(); - List<LogicalVariable> keysFirst = (keysRightBranch.containsAll(keysCurrent)) - ? keysRightBranch : keysLeftBranch; + Set<LogicalVariable> coveredVars = new ListSet<>(); + List<LogicalVariable> keysFirst = + (keysRightBranch.containsAll(originalRequiredVars)) ? keysRightBranch + : keysLeftBranch; List<LogicalVariable> keysSecond = keysFirst == keysRightBranch ? keysLeftBranch : keysRightBranch; - for (LogicalVariable r : uppreq.getColumnSet()) { + for (LogicalVariable r : originalRequiredVars) { EquivalenceClass ecSnd = eqmap.get(r); boolean found = false; int j = 0; @@ -167,33 +169,38 @@ } LogicalVariable v2 = keysSecond.get(j); EquivalenceClass ecFst = eqmap.get(v2); - for (LogicalVariable vset1 : set1) { + for (LogicalVariable vset1 : firstDeliveredVars) { if (vset1 == v2 || ecFst != null && eqmap.get(vset1) == ecFst) { - if (!covered.add(vset1)) { + if (!coveredVars.add(vset1)) { continue; } - modifuppreq.add(r); + modifiedRequiredVars.add(r); break; } } - if (covered.equals(set1)) { + if (coveredVars.equals(firstDeliveredVars)) { break; } } - if (!covered.equals(set1)) { - throw new AlgebricksException("Could not modify " + requirements - + " to agree with partitioning property " + firstDeliveredPartitioning - + " delivered by previous input operator."); + if (!coveredVars.equals(firstDeliveredVars)) { + throw new AlgebricksException( + "Could not modify the original required partitioning " + requirements + + " to agree with the partitioning property " + + firstDeliveredPartitioning + + " delivered by previous input operator. Covered vars: " + + coveredVars + ". Modified required vars: " + + modifiedRequiredVars); } - UnorderedPartitionedProperty upp2; - UnorderedPartitionedProperty rqd = (UnorderedPartitionedProperty) requirements; - if (rqd.usesPartitionsMap()) { - upp2 = UnorderedPartitionedProperty.ofPartitionsMap(modifuppreq, - rqd.getNodeDomain(), rqd.getPartitionsMap()); + UnorderedPartitionedProperty unorderedFinalRequired; + if (unorderedRequired.usesPartitionsMap()) { + unorderedFinalRequired = UnorderedPartitionedProperty.ofPartitionsMap( + modifiedRequiredVars, unorderedRequired.getNodeDomain(), + unorderedRequired.getPartitionsMap()); } else { - upp2 = UnorderedPartitionedProperty.of(modifuppreq, rqd.getNodeDomain()); + unorderedFinalRequired = UnorderedPartitionedProperty.of(modifiedRequiredVars, + unorderedRequired.getNodeDomain()); } - return new Pair<>(false, upp2); + return new Pair<>(false, unorderedFinalRequired); } case ORDERED_PARTITIONED: { throw new NotImplementedException(); diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningRequirementsCoordinator.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningRequirementsCoordinator.java index d6f4812..9a174f8 100644 --- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningRequirementsCoordinator.java +++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/IPartitioningRequirementsCoordinator.java @@ -58,51 +58,57 @@ && firstDeliveredPartitioning.getPartitioningType() == rqdpp.getPartitioningType()) { switch (rqdpp.getPartitioningType()) { case UNORDERED_PARTITIONED: { - UnorderedPartitionedProperty upp1 = + UnorderedPartitionedProperty unorderedRequired = (UnorderedPartitionedProperty) rqdpp; + UnorderedPartitionedProperty unorderedFirstDelivered = (UnorderedPartitionedProperty) firstDeliveredPartitioning; - Set<LogicalVariable> set1 = upp1.getColumnSet(); - UnorderedPartitionedProperty uppreq = (UnorderedPartitionedProperty) rqdpp; - Set<LogicalVariable> modifuppreq = new ListSet<>(); + Set<LogicalVariable> firstDeliveredVars = unorderedFirstDelivered.getColumnSet(); + Set<LogicalVariable> originalRequiredVars = unorderedRequired.getColumnSet(); + Set<LogicalVariable> modifiedRequiredVars = new ListSet<>(); Map<LogicalVariable, EquivalenceClass> eqmap = context.getEquivalenceClassMap(op); - Set<LogicalVariable> covered = new ListSet<>(); + Set<LogicalVariable> coveredVars = new ListSet<>(); - // coordinate from an existing partition property - // (firstDeliveredPartitioning) - for (LogicalVariable v : set1) { + // coordinate from an existing partition property (firstDeliveredPartitioning) + for (LogicalVariable v : firstDeliveredVars) { EquivalenceClass ecFirst = eqmap.get(v); - for (LogicalVariable r : uppreq.getColumnSet()) { - if (!modifuppreq.contains(r)) { + for (LogicalVariable r : originalRequiredVars) { + if (!modifiedRequiredVars.contains(r)) { EquivalenceClass ec = eqmap.get(r); if (ecFirst == ec) { - covered.add(v); - modifuppreq.add(r); + coveredVars.add(v); + modifiedRequiredVars.add(r); break; } } } } - if (!covered.equals(set1)) { + if (!coveredVars.equals(firstDeliveredVars)) { throw new AlgebricksException(ErrorCode.ILLEGAL_STATE, - "Could not modify " + rqdpp + " to agree with partitioning property " + "Could not modify the original required partitioning " + rqdpp + + " to agree with the partitioning property " + firstDeliveredPartitioning - + " delivered by previous input operator."); + + " delivered by previous input operator. Covered vars: " + + coveredVars + ". Modified required vars: " + + modifiedRequiredVars); } - if (modifuppreq.size() != set1.size()) { + if (modifiedRequiredVars.size() != firstDeliveredVars.size()) { throw new AlgebricksException(ErrorCode.ILLEGAL_STATE, - "The number of variables are not equal in both partitioning sides"); + "The number of variables are not equal in both partitioning sides. First delivered: " + + firstDeliveredVars + ". Modified required: " + + modifiedRequiredVars); } - UnorderedPartitionedProperty upp2; - UnorderedPartitionedProperty rqd = (UnorderedPartitionedProperty) rqdpp; - if (rqd.usesPartitionsMap()) { - upp2 = UnorderedPartitionedProperty.ofPartitionsMap(modifuppreq, - rqd.getNodeDomain(), rqd.getPartitionsMap()); + UnorderedPartitionedProperty unorderedFinalRequired; + if (unorderedRequired.usesPartitionsMap()) { + unorderedFinalRequired = UnorderedPartitionedProperty.ofPartitionsMap( + modifiedRequiredVars, unorderedRequired.getNodeDomain(), + unorderedRequired.getPartitionsMap()); } else { - upp2 = UnorderedPartitionedProperty.of(modifuppreq, rqd.getNodeDomain()); + unorderedFinalRequired = UnorderedPartitionedProperty.of(modifiedRequiredVars, + unorderedRequired.getNodeDomain()); } - return new Pair<>(false, upp2); + return new Pair<>(false, unorderedFinalRequired); } case ORDERED_PARTITIONED: { throw new NotImplementedException(); -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19531 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: ionic Gerrit-Change-Id: I270201573d5d19763bf8d68db99faa72f0b92f46 Gerrit-Change-Number: 19531 Gerrit-PatchSet: 2 Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-MessageType: merged
