You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by "Jonathan Coveney (JIRA)" <ji...@apache.org> on 2012/05/26 00:26:23 UTC
[jira] [Created] (PIG-2724) Make Tuple Iterable
Jonathan Coveney created PIG-2724:
-------------------------------------
Summary: Make Tuple Iterable
Key: PIG-2724
URL: https://issues.apache.org/jira/browse/PIG-2724
Project: Pig
Issue Type: Improvement
Reporter: Jonathan Coveney
Assignee: Jonathan Coveney
Fix For: 0.11
Seriously, is there a more annoying pattern than the following?
{code}
for (int i = 0; i < t.size(); i++) {
try {
doStuff(t.get(i));
} catch (ExecException e) {
throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
}
}
{code}
I mean yeah, you can do the following:
{code}
for (Object o : t.getAll()) {
doStuff(o);
}
{code}
But I don't even think that should be necessary. I think the following should work:
{code}
for (Object o : t) {
doStuff(o);
}
{code}
It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-1.patch
Oh wow, not chaining the exception was pure typo. Lame.
Thanks for the other comments. I went ahead and made an AbstractTuple and an AbstractTypeAwareTuple. Bam.
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Julien Le Dem (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13285888#comment-13285888 ]
Julien Le Dem commented on PIG-2724:
------------------------------------
Good idea Jon.
Some minor comments:
* chain the exception in the test:
{code}
try {
assertEquals("Element " + i, t.get(i++), o);
} catch (ExecException e) {
throw new RuntimeException(e);
}
{code}
* please add the @Override annotation to the iterator() implementations.
* To facilitate future evolutions and default implementations you can create an AbstractTuple that all tuples would extend.
For now it would have only the iterator() method implementation.
possibly it would have isNull(), toDelimitedString(), etc
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-6.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch, PIG-2724-5.patch, PIG-2724-6.patch, PIG-2724-6.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Status: Patch Available (was: Open)
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-3.patch
This removes AbstractTypeAwareTuple, though it doesn't change the compareTo/hashCode piece :)
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-4.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-2.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Prashant Kommireddi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13283935#comment-13283935 ]
Prashant Kommireddi commented on PIG-2724:
------------------------------------------
Hi Jon, patch looks good to me. Neat idea.
Minor thing, probably you could set the size of Tuple in TestTuple as that never changes in your test.
{code}
public void testTupleIterator() {
Tuple t = TupleFactory.getInstance().newTuple();
{code}
This can be changed to
{code}
public void testTupleIterator() {
Tuple t = TupleFactory.getInstance().newTuple(2000);
{code}
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-0.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Julien Le Dem (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286980#comment-13286980 ]
Julien Le Dem commented on PIG-2724:
------------------------------------
+1
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch, PIG-2724-5.patch, PIG-2724-6.patch, PIG-2724-6.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-5.patch
A minor change in toDelimitedString that handles nulls consistently.
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch, PIG-2724-5.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286222#comment-13286222 ]
Jonathan Coveney commented on PIG-2724:
---------------------------------------
I care not about the AbstractTypeAwareTuple, so I'll axe it. I knew thee well, AbstractTypeAwareTuple.
As far as the hashCode thing, you are correct of course on the typical equals contract, but for Tuples, idiomatically they all use compareTo instead. I do not think that this is an issue. I just think that we should force people to be thoughtful about compareTo/hashCode and either force both to be overriden, or neither.
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Julien Le Dem (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286177#comment-13286177 ]
Julien Le Dem commented on PIG-2724:
------------------------------------
Thanks Jonathan.
Overall, this looks good.
Some extra comments:
* In javadoc you can use the following tag whenever you want to reuse the same doc as in the implemented method (for example isNull(int))
{code}
/**
* {@inheritDoc}
*/
{code}
* I would not necessarily create AbstractTypeAwareTuple unless there is something to add in it
* It looks like the toDelimitedString() in DefaultTuple was doing something different (nnull would be written differently) than the new default one in AbstractTuple. We may want to make sure we keep the same behavior in case someone is parsing it. You could add a test for this.
* hashCode() could be in AbstractTuple as well as it just needs an object iterator (that you just provided!)
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-4.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-6.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch, PIG-2724-5.patch, PIG-2724-6.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-7.patch
rebased and committed
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch, PIG-2724-5.patch, PIG-2724-6.patch, PIG-2724-6.patch, PIG-2724-7.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch, PIG-2724-3.patch, PIG-2724-4.patch, PIG-2724-4.patch, PIG-2724-5.patch, PIG-2724-6.patch, PIG-2724-6.patch, PIG-2724-7.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286183#comment-13286183 ]
Jonathan Coveney commented on PIG-2724:
---------------------------------------
- Thanks for the heads up on the javadoc
- Yeah, I was thinking about pushing in hashCode as well, but I don't like the idea of pushing in hashCode if we don't push in compareTo as well... either we have default behavior for both, or neither, IMHO.
- I will have DefaultTuple override it and do what it used to do, and leave AbstractTuple's as the general.
- AbstractTypeAwareTuple let's you merge together AbstractTuple+TypeAwareTuple interface. It feels cleaner than forcing classes to do that themselves?
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Updated] (PIG-2724) Make Tuple Iterable
Posted by "Jonathan Coveney (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Coveney updated PIG-2724:
----------------------------------
Attachment: PIG-2724-1.patch
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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
[jira] [Commented] (PIG-2724) Make Tuple Iterable
Posted by "Julien Le Dem (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286220#comment-13286220 ]
Julien Le Dem commented on PIG-2724:
------------------------------------
* np
* You are correct but your argument for hashCode() also applies to equals(). The contract is that o1.equals(o2) => o1.hashCode() == o2.hashCode() (not the other way around). compareTo() is separate but of course o1.compareTo(o2) == 0 <=> o1.equals(o2)
Usually hashCode and equals are implemented together
If the individual objects are not comparable it is hard to provide a default compareTo() implementation.
Factor out what makes sense to you. Some warning about that in the javadoc?
* Sounds good
* I don't think factoring the implements statement is worth the extra AbstractTypeAwareTuple class. It doesn't really improve readability.
> Make Tuple Iterable
> -------------------
>
> Key: PIG-2724
> URL: https://issues.apache.org/jira/browse/PIG-2724
> Project: Pig
> Issue Type: Improvement
> Reporter: Jonathan Coveney
> Assignee: Jonathan Coveney
> Fix For: 0.11
>
> Attachments: PIG-2724-0.patch, PIG-2724-1.patch, PIG-2724-1.patch, PIG-2724-2.patch
>
>
> Seriously, is there a more annoying pattern than the following?
> {code}
> for (int i = 0; i < t.size(); i++) {
> try {
> doStuff(t.get(i));
> } catch (ExecException e) {
> throw new RuntimeException("BUT I THOUGHT PIG WAS SINGLETHREADED", e);
> }
> }
> {code}
> I mean yeah, you can do the following:
> {code}
> for (Object o : t.getAll()) {
> doStuff(o);
> }
> {code}
> But I don't even think that should be necessary. I think the following should work:
> {code}
> for (Object o : t) {
> doStuff(o);
> }
> {code}
> It's a shame we can't give a default implementation (I either shake my fist that this won't be possible until Java 8 OR that Tuple is an interface and not an abstract class). Either way, I even added test! WOAH.
> Thoughts?
--
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