You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by "Swati Jain (JIRA)" <ji...@apache.org> on 2010/07/21 11:16:51 UTC

[jira] Created: (PIG-1510) Add `deepCopy` for LogicalExpressions

Add `deepCopy` for LogicalExpressions
-------------------------------------

                 Key: PIG-1510
                 URL: https://issues.apache.org/jira/browse/PIG-1510
             Project: Pig
          Issue Type: New Feature
          Components: data
    Affects Versions: 0.8.0
            Reporter: Swati Jain
            Assignee: Swati Jain
             Fix For: 0.8.0


It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
* It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
* A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).

The usage would look like the following:
{noformat}
LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
{noformat}

An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Swati Jain updated PIG-1510:
----------------------------

    Status: Patch Available  (was: Open)

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Daniel Dai (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890882#action_12890882 ] 

Daniel Dai commented on PIG-1510:
---------------------------------

When I work on 1178, I already added that utility since I will use it in my code. You can use your deepCopy currently, once I submit patch for 1178, you can review the code and see if we can share this code.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Swati Jain updated PIG-1510:
----------------------------

    Attachment: deepCopy.patch

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Swati Jain updated PIG-1510:
----------------------------

    Status: Patch Available  (was: In Progress)

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Swati Jain (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890834#action_12890834 ] 

Swati Jain commented on PIG-1510:
---------------------------------

Hi Daniel, 

I am ready with this patch and I am using it in ticket 1494. I'll add the patch over here very soon. Please review that patch. If you want I can also add that patch to PIG-1178 too.


> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Daniel Dai (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890777#action_12890777 ] 

Daniel Dai commented on PIG-1510:
---------------------------------

Yes, this should be a useful functionality. It will be included in the next patch for PIG-1178. I will submit a patch very soon.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Swati Jain updated PIG-1510:
----------------------------

    Patch Info: [Patch Available]

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Swati Jain (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890899#action_12890899 ] 

Swati Jain commented on PIG-1510:
---------------------------------

I just attached my patch, please see if it is significantly different from what you are planning to add and let me know if it is so I can redesign if required. Also, if this is the same as what you did, and If it helps your work, I could add this as a separate change.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12893568#action_12893568 ] 

Hadoop QA commented on PIG-1510:
--------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12450096/deepCopy.patch
  against trunk revision 980276.

    +1 @author.  The patch does not contain any @author tags.

    -1 tests included.  The patch doesn't appear to include any new or modified tests.
                        Please justify why no tests are needed for this patch.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    -1 findbugs.  The patch appears to introduce 2 new Findbugs warnings.

    -1 release audit.  The applied patch generated 435 release audit warnings (more than the trunk's current 406 warnings).

    -1 core tests.  The patch failed core unit tests.

    -1 contrib tests.  The patch failed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/384/testReport/
Release audit warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/384/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/384/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/384/console

This message is automatically generated.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Swati Jain updated PIG-1510:
----------------------------

    Status: In Progress  (was: Patch Available)

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Daniel Dai (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12899197#action_12899197 ] 

Daniel Dai commented on PIG-1510:
---------------------------------

Hi, Swati,
Do we also make a copy of all the instance variables? Such as fieldSchema, ConstantExpression.val, etc?

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Swati Jain updated PIG-1510:
----------------------------

    Attachment: deepCopy.patch

Revised patch after merging changes. Also wrote a unit test.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Daniel Dai (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12899498#action_12899498 ] 

Daniel Dai commented on PIG-1510:
---------------------------------

You are right about fieldSchema and uidOnlyFieldSchema. For ConstantExpression.val etc, if you only copy the reference, you need to make sure they are readonly in your use case. If that is the case, +1 for the patch.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Commented: (PIG-1510) Add `deepCopy` for LogicalExpressions

Posted by "Swati Jain (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12899282#action_12899282 ] 

Swati Jain commented on PIG-1510:
---------------------------------

We make a copy of all instance variables, including fieldSchema variables in expression classes. Exceptions are:
a) Object val in ConstantExpression. We just copy a reference to the constant object value since the object is constant by definition.
b) `fieldSchema` and `uidOnlyFieldSchema` in LogicalExpression.java since these can be constructed when required from the expression copy by invoking `getFieldSchema` and don't need to be copied.

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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


[jira] Updated: (PIG-1510) Add `deepCopy` for LogicalExpressions

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

Daniel Dai updated PIG-1510:
----------------------------

          Status: Resolved  (was: Patch Available)
    Hadoop Flags: [Reviewed]
      Resolution: Fixed

Patch committed to trunk. Thanks Swati for contributing!

> Add `deepCopy` for LogicalExpressions
> -------------------------------------
>
>                 Key: PIG-1510
>                 URL: https://issues.apache.org/jira/browse/PIG-1510
>             Project: Pig
>          Issue Type: New Feature
>          Components: data
>    Affects Versions: 0.8.0
>            Reporter: Swati Jain
>            Assignee: Swati Jain
>             Fix For: 0.8.0
>
>         Attachments: deepCopy.patch, deepCopy.patch
>
>
> It would be useful to have a way to `deepCopy` an expression. `deepCopy` will create a new object so that changes made to one object will not reflect in the copy. There are 2 reasons why we don't override clone.
> * It may be better to use `deepCopy` since the copy semantics are explicit (since deepCopy may be expensive).
> * A second important reason for defining `deepCopy` as a separate routine is that it can be passed a plan as an argument which will be updated as the expression is copied (through plan.add and plan.connect).
> The usage would look like the following:
> {noformat}
> LogicalExpressionPlan logicalPlan = new LogicalExpressionPlan();
> LogicalExpression copyExpression = origExpression.deepCopy( logicalPlan );
> {noformat}
> An immediate motivation for this would be for constructing the expressions that constitute the CNF form of an expression.

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