hadoop-pig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shravan Matthur Narayanamurthy (JIRA)" <j...@apache.org>
Subject [jira] Commented: (PIG-161) Rework physical plan
Date Tue, 01 Apr 2008 09:10:25 GMT

    [ https://issues.apache.org/jira/browse/PIG-161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12584072#action_12584072

Shravan Matthur Narayanamurthy commented on PIG-161:

Thank you for the input Pi. Responses in line...

    * The proposal in wiki says "Make the model entirely push-based" but this implementation
is pull-based. What is the real direction on this?
Hmm, Interesting observation. We never realized there could be such a grave error lurking
in the documentation. The consensus was to have a single threaded, pull based model. The current
model is push based. We need to change this.
@Alan: I am not sure how this happened?!
    * This comment is obvious. Those dummy type instances in POFilter can be made static.
If they are used all over the place, we should move them to a more generic parent class.
Its a good idea. I think we can introduce these into PhysicalOperator as static dummy types.
But we should also make this known to anyone implementing physical operators so that these
can be used. I will move them.
    * visit() should not throw ParseException. Look at PIG-169 for example. In some cases
we use visit later in the processing after parsing. If we expect ParseException when parsing
then we can just wrap in ParseException.
This is a side effect of the Operator's visit method throwing ParseException. But I think
you are right. We should throw some other generic exception than a ParseException.
@Alan: Could you please comment on this?
    * Therefore doAllPredecessors() and doAllPredecessors() also should not throw ParseException
Same as the previous one.
    * GreaterThanExpr, LessThanExpr, GTOrEqualToExpr, NotEqualToExpr + a few more are 90%
the same. Is it possible to have a common abstract class for them?
Well, if you can think of a better way it would be great. The main thing that nfluenced this
choice was that we wanted to minimize the branching constructs. The moment we introduce an
abstract class, we would need them. This was done because the physical side assumes that the
type checker will provide appropriate type information and we can use them inside expressions
to avoid branching.
    * This is subjective. The name depthFirst() for me is a bit mis-leading because this specific
depthFirst() doesn't walk over already seen nodes.
I concur. It is depthFirst in a restricted sense. Again the same search can be done in both
directions either from the top or from the bottom. If we choose to change this in OperatorPlan,
I am fine with this.
@Alan: Your comments needed.
    * From this "The input model assumes that it can either be taken from an operator or can
be attached directly to this operator" could you explain more what attachinput and input in
PhysicalOperator do?
PhysicalOperator is the base class for all the operators. Since each operator needed the same
input model, I just refactored it into the base class. So the input model is an artifact of
the execution model. Since we decided have attribute plans, the model that we converged to
was that the attribute plans be separated from the top level operator plan and scoped within
the operator that it is an attribute of. So when the top level operator pulls tuples from
its input operator, it attaches these tuples to the attribute plan and calls the getNext on
the root operator of the plan. The attach of the attribute plan in turn uses attachInput fn
the leaf operators. I hope I am clear. If not please let me know. I wil rephrase it.

> Rework physical plan
> --------------------
>                 Key: PIG-161
>                 URL: https://issues.apache.org/jira/browse/PIG-161
>             Project: Pig
>          Issue Type: Sub-task
>            Reporter: Alan Gates
>            Assignee: Alan Gates
>         Attachments: Phy_AbsClass.patch
> This bug tracks work to rework all of the physical operators as described in http://wiki.apache.org/pig/PigTypesFunctionalSpec

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

View raw message