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-158) Rework logical plan
Date Thu, 20 Mar 2008 21:47:25 GMT

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

Santhosh Srinivasan commented on PIG-158:
-----------------------------------------

Shravan,

See my replies inline with [Santhosh]. Thanks for the comments.

1) For the visitor, can we not type the Operator with the PhysicalPlan, and the PlanVisitor
with Operator and the OperatorPlan so that a lot of those instanceof checks in the visit methods
and in LOVisitor can be avoided.

[Santhosh] Let me separate the comment into two parts
Part 1: Avoiding the instanceof checks- The instanceof checks are present only in the visit
method of the operators.
Part 2: Having the visit method in Physical

My attempt at getting the compiler to typecheck was a failure at the end. What I tried out
follows:

Operator.java:

public abstract <T extends PlanVisitor> void visit(T v) throws ParseException;

LOCogroup.java:

public <T extends LOVisitor> void visit(T v) throws ParseException {v.visit(this)};

ant compile fails with the following error messages:

    [javac] Z:\src_pig\pig\branches\types\src\org\apache\pig\impl\logicalLayer\LOCogroup.java:31:
org.apache.pig.impl.logicalLayer.LOCogroup is not abstract and does not override abstract
method <T>visit(T) in org.apache.pig.impl.plan.Operator
    [javac] public class LOCogroup extends LogicalOperator {
    [javac]        ^
    [javac] Z:\src_pig\pig\branches\types\src\org\apache\pig\impl\logicalLayer\LOCogroup.java:112:
method does not override a method from its superclass
    [javac]     @Override


However, if run ant compile again, without making any changes, it succeeds. I do not know
how to account for this peculiar behaviour

The other option to get ant to work successfully without any magic is the following:

Operator.java:

public abstract <T extends PlanVisitor> void visit(T v) throws ParseException;

LOCogroup.java:

public <T extends PlanVisitor> void visit(T v) throws ParseException {v.visit(this)};
//requires instanceof to check for LOVisitor


Here, instanceof checks are required to ensure that v is of type LOVisitor.

If someone can shed shome light on this, it would be help us use Java's type checking instead
of the instanceof operator.


2) Can you check the getSchema method of LOSort?

[Santhosh] Thanks - there was a duplication of the check for hasNext(). I will fix that

3) Why do we have an LOEval?

[Santhosh] LOEval will be removed

4) In LOGenerate, why is mProjections a List<LogicalOperator> instead of List<ExpressionOperator>?

[Santhosh] Good question. Generate has projections which can be modeled as LogicalOperators
instead of ExpressionOperators. I left it at the higher abstraction.

5) I think you need to look more into aliases. In many places you set it to null but I felt
there can be aliases attached to it. I am not sure abt this. But pls check.

[Santhosh] Are you referring to mSchema = null ? Since the method getSchema can throw an exception,
I am ensuring that the state is consistent by setting it to null and resetting the flag for
schema computation.

6) In LOForeach why do you have a List<LogicalOperators> mOperators? Instead I was hoping
that there will be a LogicalPlan. Then you will have LOGenerate as the leaf node.

[Santhosh] Good question again. Since foreach allows one level nesting, my interpretation
was to have a list of logical operators as the attribute of foreach. I am not checking if
LOGenerate is the last operator (or leaf node) in this list. In the method getSchema, I am
making this assumption (also documented in code) and returning the schema of the last operator.
In addition, if foreach allows multiple levels of nesting in the future, one of the logical
operators in the list can again be a foreach.

I see your point of view too. Can someone review my analysis?

Santhosh

> Rework logical plan
> -------------------
>
>                 Key: PIG-158
>                 URL: https://issues.apache.org/jira/browse/PIG-158
>             Project: Pig
>          Issue Type: Sub-task
>          Components: impl
>            Reporter: Alan Gates
>            Assignee: Alan Gates
>         Attachments: logical_operators.patch
>
>
> Rework the logical plan in line with http://wiki.apache.org/pig/PigExecutionModel

-- 
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