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