hadoop-pig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Santhosh Srinivasan (JIRA)" <j...@apache.org>
Subject [jira] Commented: (PIG-331) Combiner needs to be used in the types branch
Date Tue, 29 Jul 2008 21:53:31 GMT

    [ https://issues.apache.org/jira/browse/PIG-331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12617972#action_12617972
] 

Santhosh Srinivasan commented on PIG-331:
-----------------------------------------

   - File: src/org/apache/pig/backend/local/executionengine/LocalJob.java

      * In the anonymous Iterator that is returned, I noticed that the public member 

variables are not initialized but they are used in the hasNext() method.

{code}
return new Iterator<Tuple>() {

+            Tuple   t;
+            boolean atEnd;
+            public boolean hasNext() {
+                if (atEnd)
+                    return false;
+                try {
+                    if (t == null)
{code}


   - File: 

src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/CombinerOptimizer.java

      * Minor Comment: the log message indicates a leaf while the checks are being made for


the roots.

{code}
 if (reduceRoots.size() != 1) {
+            log.warn("Expected reduce to have single leaf");
+            return;
+        }

{code}

      * Minor Comment: The condition to check for the existence of at least one algebraic


and no instances of non-algebraic types is probably better written using && instead
of ||

Method: +    private List<ExprType> algebraic(


{code}
//current code
if (!atLeastOneAlgebraic || !noNonAlgebraics) return null;

//more readable code?

if (atLeastOneAlgebraic && noNonAlgebraics) return null;

{code}


   - File: src/org/apache/pig/impl/plan/OperatorPlan.java

      * Major Comment:

The new replace(oldNode, newNode) method is similar to the private replaceNode(src, 

replacement, dst, multiMap) method added as part of 

https://issues.apache.org/jira/browse/PIG-290. The difference lies in the method signature


and the access specifier.


{code}
public void replace(E oldNode, E newNode) throws PlanException {
//Pig-290 change below
private boolean replaceNode(E src, E replacement, E dst, MultiMap<E, E> multiMap) 
{code}


   - File: src/org/apache/pig/impl/util/Pair.java

      * Minor Comment:
Do we need equals() and hashCode() methods to make the class usable in containers?

   - File: 

   - File: 

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserCo

mparisonFunc.java

      * Major Comment:

In the clone() method, the ComparisonFunc func is not cloned


   - File: 

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFu

nc.java

   * Major Comment:

In the clone method, the final two constructor arguments are in the wrong order. POUserFunc


expects FuncSpec, EvalFunc while the clone method uses null, funcSpec.clone. This should not


compile in the first place.

{code}
+        POUserFunc clone = new POUserFunc(new OperatorKey(mKey.scope, 
+            NodeIdGenerator.getGenerator().getNextNodeId(mKey.scope)),
+            requestedParallelism, null, funcSpec.clone());
{code}

   - File: 

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/BinaryEx

pressionOperator.java

   * Major Comment:

In the cloneHelper method, the operator's resultType is not set. In the corresponding 

cloneHelper method in UnaryExpressionOperator, the restultType is set. The resultType is 

also set in PhysicalOperator's cloneHelper method.

{code}
//in UnaryExpressionOperator.java's cloneHelper

+        resultType = op.getResultType();
{code}

   - File: 

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/PODistin

ct.java

   * Minor Comment:

The clone method is missing in PODistinct. Is this due to the re-write of distinct in 

LocalRearrange?

> Combiner needs to be used in the types branch
> ---------------------------------------------
>
>                 Key: PIG-331
>                 URL: https://issues.apache.org/jira/browse/PIG-331
>             Project: Pig
>          Issue Type: Bug
>    Affects Versions: types_branch
>            Reporter: Alan Gates
>            Assignee: Alan Gates
>            Priority: Critical
>             Fix For: types_branch
>
>         Attachments: combiner.patch
>
>
> The initial implementation in the types branch does not make use of the combiner.  For
performance, it needs to make use of the combiner.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message