pig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Gates (JIRA)" <j...@apache.org>
Subject [jira] Commented: (PIG-161) Rework physical plan
Date Tue, 01 Apr 2008 19:04:24 GMT

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

Alan Gates commented on PIG-161:

Alan's responses to Shravan's responses to Pi:

   * The proposal in wiki says "Make the model entirely push-based" but this implementation
is pull-based. What is the real direction on this?
      [shrav] 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?!
	  Alan>> I don't remember, but the relevant point is that the docs need updated (in
many more instances than just this).  It's on my todo list, but I haven't gotten to it yet.

    * 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.
      [shrav] 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?
	  Alan>> I agree that ParseException is wrong.  Based on http://wiki.apache.org/pig/PigDeveloperCookbook,
it appears the correct exception to throw would be a FrontendException.  Perhaps we should
extend this to have VisitException that could then be thrown by all visitors.  If we're in
agreement on this I can take on the task of changing PlanVisitor and its children.

    * This is subjective. The name depthFirst() for me is a bit mis-leading because this specific
depthFirst() doesn't walk over already seen nodes.
      [shrav] 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.
	  Alan>> My rules for function nomenclature are 1) it should be reasonably descriptive;
2) it should not be confusing; 3) it should not be too long (not everyone uses an IDE that
fills in function names, plus it makes it hard to fit much on one line).  Shravan and Pi seem
to be in agreement that depthFirst violates rule 2.  If you suggest a better name that fits
all three rules I'm cool with that.

> 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, pogenerate.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