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/02/27 15:35:51 UTC

[jira] Created: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

custom storage ( LOAD with USING) doesn't work with inner classes
-----------------------------------------------------------------

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


I might be trivial but it has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song commented on PIG-126:
-----------------------------

Agree with both of you.
Let's close this bug.

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song commented on PIG-126:
-----------------------------

My problem was with static inner classes. The scenario was that I was doing a Test Case and I wanted a dummy Storage that returns me a specific set of data. I didn't want to let those Storage classes messy around so I declare them as static inner classes.

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Updated: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Johannes Zillmann updated PIG-126:
----------------------------------

    Attachment: PIG-126-test.patch

Was curious cause i stumbled over the same issue some day. I created a test case which uses Load/StoreFuncs of different class-types. 
I think using inner classes as Store/LoadFunc can be fixe by instantiating the "parent-class" which then have to have a default constructor (the method testInnerClass() shows how to instantiate a inner class).

So also this should be fixable with a fair amount of time i wonder if there are situations where this is really useful! 

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Updated: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song updated PIG-126:
------------------------

    Attachment: pig126_UnitTest2.patch

I've modified the unit test a bit to handle expected exceptions in negative cases.

This change is only adding another unit test to the code base. Nothing will be affected.

Tested and ready to be committed.

Thanks Johannes for the initial code.

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch, pig126_UnitTest2.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

Posted by "Johannes Zillmann (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574175#action_12574175 ] 

Johannes Zillmann commented on PIG-126:
---------------------------------------

Hi Pi,

that is exactly the use case i find very useful too. And i think i did not work for me one time. But this test shows it does...
And i cannot reproduce the non-working situation ;)
Can you have a look in the appended testcase and into your testcase to see if there is any difference ?


> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song commented on PIG-126:
-----------------------------

Johannes,
Firstly, thank a lot for helping me track down this problem. Your patch has helped me scope down the possibilities.

I have found out that in order to refer to a static inner class, we have to say "OuterClass1$InnerClass2()" as opposed to the intuitive  "OuterClass1.InnerClass2()" (This apparently also happens when you do instantiate using reflection). 

So that in your patch when you use LoadFuncClass.getName(), you get the right name all the time.

This, therefore, may not be a bug. Though personally I prefer "OuterClass1.InnerClass2()".  Should we make both forms usable?

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

Posted by "Olga Natkovich (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574772#action_12574772 ] 

Olga Natkovich commented on PIG-126:
------------------------------------

I think even to give up a 1% of performance a change/feature should have a significant use case for large number of users.

I agree with Ben. I don't think we need to "fix" how java deals with inner classes but we do need to document that.

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574601#action_12574601 ] 

Benjamin Reed commented on PIG-126:
-----------------------------------

I'm not very happy with how Sun did inner classes in Java, but it is what it is. I'm not sure how to address this issue. Making OuterClass1.InnerClass2 work would be hard since there can be multiple layers of nesting, so the code would be ugly and not very efficient.

Could this be solved with just documentation? If so, where? Part of the problem is that this is straight Java weirdness. It doesn't really have anything to do with Pig.

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574780#action_12574780 ] 

Benjamin Reed commented on PIG-126:
-----------------------------------

No, this is not a performance problem at all. We resolve the class once when we parse. There would be no perceivable performance impact.

This really is more about keeping code simple and getting rid of weird corner cases.

I can see how you see it as losing some simplicity, but if you are using inner classes there are things to be aware of anyway, such as making the inner classes static. It also makes problem determination a lot simpler. If you specify a.b.c() as a function and it is really a.b$c, some poor guy is going to look for a.b.c.class file and not find it. I completely agree that it is ugly to require the $, but it is a Java requirement and adding layers to cover up ugliness often results in more confusion. (Both for those looking at the code that implements the layer and for those sitting up on top when something goes wrong.)

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Closed: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Olga Natkovich closed PIG-126.
------------------------------

    Resolution: Fixed

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Reopened: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song reopened PIG-126:
-------------------------


Sorry, my mistake. We should incorporate the unit test from Johannes into our source base before we close.

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIG-126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573353#action_12573353 ] 

Benjamin Reed commented on PIG-126:
-----------------------------------

Was it a static inner class? What exception did you get?

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Commented: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song commented on PIG-126:
-----------------------------

Ben, 
Can I ask you a question? Isn't what we're doing here in Pig is trading performance for simplicity?
I think your claim was right. This will not be so efficient to support inner classes. *But* if that performance hit doesn't go beyond 1% boundary, I think it should be sufficient.


> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>         Attachments: PIG-126-test.patch
>
>
> It might be trivial but this has held me up for quite a while.

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


[jira] Updated: (PIG-126) custom storage ( LOAD with USING) doesn't work with inner classes

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

Pi Song updated PIG-126:
------------------------

    Description: It might be trivial but this has held me up for quite a while.  (was: I might be trivial but it has held me up for quite a while.)

> custom storage ( LOAD with USING) doesn't work with inner classes
> -----------------------------------------------------------------
>
>                 Key: PIG-126
>                 URL: https://issues.apache.org/jira/browse/PIG-126
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.1.0
>            Reporter: Pi Song
>            Priority: Minor
>
> It might be trivial but this has held me up for quite a while.

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