You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by "Pi Song (JIRA)" <ji...@apache.org> on 2008/07/03 16:12:45 UTC

[jira] Created: (PIG-290) LOCross output schema is not right

LOCross output schema is not right
----------------------------------

                 Key: PIG-290
                 URL: https://issues.apache.org/jira/browse/PIG-290
             Project: Pig
          Issue Type: Bug
          Components: impl
    Affects Versions: types_branch
            Reporter: Pi Song


>From the schema generation code:-

{noformat}
        List<LogicalOperator> inputs = mPlan.getPredecessors(this);

            for (LogicalOperator op : inputs) {
                    // Create schema here
            }
{noformat}

The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Updated: (PIG-290) LOCross output schema is not right

Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Santhosh Srinivasan updated PIG-290:
------------------------------------

    Fix Version/s: types_branch

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612123#action_12612123 ] 

Pi Song commented on PIG-290:
-----------------------------

The patch doesn't seem to relate to the bug.
Are you posting in the right Jira?

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12614363#action_12614363 ] 

Pi Song commented on PIG-290:
-----------------------------

I wouldn't rely on a side effect instead of the correct model.

The rewiring problem seems to be universal. One solution is we can have a set of utility methods for inserting/cutting nodes in logical plans that always guarantees correct wiring.

Now our votes are 1 to 1. I'll ask Alan to give the third vote.

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12614115#action_12614115 ] 

Santhosh Srinivasan commented on PIG-290:
-----------------------------------------

The edge ordering in the implementation is the order in which the connections were made. This is a side effect of using a list to store the edges. While this does not appear to be a clean solution, the scope of changing the definition of the logical operators LOCross and LOCogroup is widespread. The changes will affect the optimizer which has to re-wire the inputs when operators are moved around in the graph.

For now, I would go with the proposed change. We can fix the issue as an enhancement when we merge from types branch to trunk.

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Issue Comment Edited: (PIG-290) LOCross output schema is not right

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613914#action_12613914 ] 

pi_song edited comment on PIG-290 at 7/16/08 4:30 AM:
------------------------------------------------------

Sorry, I had a look at this a few days ago but forgot to reply.
I think instead of relying on  edge ordering in the graph (which is, in fact, not defined), we'd better maintain the order inside LO.

What is your opinion on this?

      was (Author: pi_song):
    Sorry, I had a look at this a few days ago but forgot to reply.
I think instead of relying on  edge ordering in the graph (which is, in fact, not defined), we'd better maintain the order inside LO.

What is your opinion of this?
  
> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Resolved: (PIG-290) LOCross output schema is not right

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pi Song resolved PIG-290.
-------------------------

    Resolution: Fixed

Checked in.
Thanks Santhosh!

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Assigned: (PIG-290) LOCross output schema is not right

Posted by "Alan Gates (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alan Gates reassigned PIG-290:
------------------------------

    Assignee: Santhosh Srinivasan

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613914#action_12613914 ] 

Pi Song commented on PIG-290:
-----------------------------

Sorry, I had a look at this a few days ago but forgot to reply.
I think instead of relying on  edge ordering in the graph (which is, in fact, not defined), we'd better maintain the order inside LO.

What is your opinion of this?

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Updated: (PIG-290) LOCross output schema is not right

Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Santhosh Srinivasan updated PIG-290:
------------------------------------

    Attachment: insert_between.patch

The insert_between.patch fixes the following:

1. Changes the implementation of insertBetween in OperatorPlan.java so that the ordering of the predecessors is not changed

2. Changed log.info in LOForeach getSchema() to log.debug. One of my earlier patches had the log.info

3. Removed a printPlan use in TestLogicalPlanBuilder.java

The unit tests that still fail are:

    [junit] Running org.apache.pig.test.TestEvalPipeline
    [junit] Tests run: 8, Failures: 0, Errors: 1, Time elapsed: 141.926 sec
    [junit] Test org.apache.pig.test.TestEvalPipeline FAILED

    [junit] Running org.apache.pig.test.TestFilterOpNumeric
    [junit] Tests run: 8, Failures: 0, Errors: 1, Time elapsed: 56.446 sec
    [junit] Test org.apache.pig.test.TestFilterOpNumeric FAILED

    [junit] Running org.apache.pig.test.TestStoreOld
    [junit] Tests run: 3, Failures: 0, Errors: 2, Time elapsed: 39.782 sec
    [junit] Test org.apache.pig.test.TestStoreOld FAILED


> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Pi Song (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12614736#action_12614736 ] 

Pi Song commented on PIG-290:
-----------------------------

will commit this once I figure out why a lot more tests are failing today.

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612205#action_12612205 ] 

Santhosh Srinivasan commented on PIG-290:
-----------------------------------------

I should have explained better on my previous comment. The ordering of the nodes in the getPredecessor and getSuccessor calls are preserved as long as the nodes are not moved around using the disconnect, connect and insertBetween calls. I have fixed the insertBetween calls to preserve the right order.

If the original order was A->C and A->D if a new node B was inserted between A and C, then previously you would end up with A->D and then A-B->C. Now you will have A->B->C and A->D, thus preserving the order for the getPredecessor and getSuccessor calls.

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Alan Gates (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12614547#action_12614547 ] 

Alan Gates commented on PIG-290:
--------------------------------

I don't think it's quite fair to characterize this is as relying a on side effect.  Santhosh's changes guarantee that side effect and then rely on it.  I agree that the resulting API is a little confusing (using disconnect/connect will have a different affect that using replaceNode).  That needs to be very clearly documented.  But I'm sure it would be any better if operators had inputs and anyone rearranging the tree had to remember to reconnect the inputs.  In that case the programmer would have to remember to reset the inputs, which is just as hard as remembering to use the correct API function.

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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


[jira] Commented: (PIG-290) LOCross output schema is not right

Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613662#action_12613662 ] 

Santhosh Srinivasan commented on PIG-290:
-----------------------------------------

Can we commit this patch, if the review and the explanation are fine?

Thanks,
Santhosh

> LOCross output schema is not right
> ----------------------------------
>
>                 Key: PIG-290
>                 URL: https://issues.apache.org/jira/browse/PIG-290
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: types_branch
>            Reporter: Pi Song
>            Assignee: Santhosh Srinivasan
>             Fix For: types_branch
>
>         Attachments: insert_between.patch
>
>
> From the schema generation code:-
> {noformat}
>         List<LogicalOperator> inputs = mPlan.getPredecessors(this);
>             for (LogicalOperator op : inputs) {
>                     // Create schema here
>             }
> {noformat}
> The output schema is generated based on inputs determined in the logical plan. However,  mPlan.getPredecessors() doesn't always preserve the right order  (A x B and B x A result in different schemas). I suggest maintaining mInputs variable in LOCross (as it used to be) to resolve this issue.

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