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

Jonathan Coveney commented on PIG-2600:
---------------------------------------

So much refactoring is necessary in so many places...oy haha. As far as how 
much code to wrap...yeah, that's just style at this point. Not worth nitpicking 
over. I do have a couple of last comments...

In VALUELIST (and a couple of other places I think)
{code}
+        try {
+            //Input must be of type Map. This is verified at compile time
+            m = (Map<String, Object>)(input.get(0));
+        } catch (ExecException e) {
+            throw e;
+        }
{code}

the catch shouldn't be necessary anymore, since exec throws IOException and 
ExecException is an IOException.

More of style point, but in KEYSET:
{code}
+        Iterator<String> iter = keySet.iterator();
+        DataBag bag = BAG_FACTORY.newDefaultBag();
+        //Add keyset elements to bag
+        while(iter.hasNext()) {
+            Tuple t = TUPLE_FACTORY.newTuple(iter.next());
+            bag.add(t);
+        }
{code}

could be rewritten as
{code}
DataBag bag = BAG_FACTORY.newDefaultBag();
for (String s : keySet) {
    Tuple t = TUPLE_FACTORY.newTuple(iter.next());
    bag.add(t);
}
{code}

IMHO using the for construct with iterators is a lot clearer, but it's not a 
big deal... if there weren't other things I wouldn't have bother mentioning it, 
but here it is haha.

- You can have the doInverse method throw ExecException, then instead of 
throwing RuntimeException, throw an ExecException, which you won't have to 
catch since exec throws IOException. Also, the exception message is a little 
misleading, as the error isn't that of getting and unknown primitive (primitive 
implies int etc), but just a wrong type in general.

That's it though. Should be good for committing after that.

Thanks for the work!
                
> Better Map support
> ------------------
>
>                 Key: PIG-2600
>                 URL: https://issues.apache.org/jira/browse/PIG-2600
>             Project: Pig
>          Issue Type: Improvement
>            Reporter: Jonathan Coveney
>            Assignee: Prashant Kommireddi
>             Fix For: 0.11
>
>         Attachments: PIG-2600.patch, PIG-2600_2.patch, PIG-2600_3.patch, 
> PIG-2600_4.patch, PIG-2600_5.patch, PIG-2600_6.patch
>
>
> It would be nice if Pig played better with Maps. To that end, I'd like to add 
> a lot of utility around Maps.
> - TOBAG should take a Map and output {(key, value)}
> - TOMAP should take a Bag in that same form and make a map.
> - KEYSET should return the set of keys.
> - VALUESET should return the set of values.
> - VALUELIST should return the List of values (no deduping).
> - INVERSEMAP would return a Map of values => the set of keys that refer to 
> that Key
> This would all be pretty easy. A more substantial piece of work would be to 
> make Pig support non-String keys (this is especially an issue since UDFs and 
> whatnot probably assume that they are all Integers). Not sure if it is worth 
> it.
> I'd love to hear other things that would be useful for people!

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to