You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oodt.apache.org by Ricky Nguyen <ri...@gmail.com> on 2012/03/23 01:10:25 UTC
Review Request: filemgr query throws NPE when sorting by key with possibly
empty values
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/
-----------------------------------------------------------
Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
Summary
-------
putting nulls last
This addresses bug OODT-429.
https://issues.apache.org/jira/browse/OODT-429
Diffs
-----
trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
Diff: https://reviews.apache.org/r/4458/diff
Testing
-------
wrote unit test
Thanks,
Ricky
Re: Review Request: filemgr query throws NPE when sorting by key with
possibly empty values
Posted by brian Foster <ho...@juno.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/#review6371
-----------------------------------------------------------
Ship it!
LGTM
- brian
On 2012-03-23 21:19:16, Ricky Nguyen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4458/
> -----------------------------------------------------------
>
> (Updated 2012-03-23 21:19:16)
>
>
> Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
>
>
> Summary
> -------
>
> putting nulls last
>
>
> This addresses bug OODT-429.
> https://issues.apache.org/jira/browse/OODT-429
>
>
> Diffs
> -----
>
> trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
> trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4458/diff
>
>
> Testing
> -------
>
> wrote unit test
>
>
> Thanks,
>
> Ricky
>
>
Re: Review Request: filemgr query throws NPE when sorting by key with
possibly empty values
Posted by Chris Mattmann <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/#review6348
-----------------------------------------------------------
Ship it!
LGTM.
trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java
<https://reviews.apache.org/r/4458/#comment13773>
Agreed.
- Chris
On 2012-03-23 21:19:16, Ricky Nguyen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4458/
> -----------------------------------------------------------
>
> (Updated 2012-03-23 21:19:16)
>
>
> Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
>
>
> Summary
> -------
>
> putting nulls last
>
>
> This addresses bug OODT-429.
> https://issues.apache.org/jira/browse/OODT-429
>
>
> Diffs
> -----
>
> trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
> trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4458/diff
>
>
> Testing
> -------
>
> wrote unit test
>
>
> Thanks,
>
> Ricky
>
>
Re: Review Request: filemgr query throws NPE when sorting by key with
possibly empty values
Posted by brian Foster <ho...@juno.com>.
> On 2012-03-26 21:06:00, brian Foster wrote:
> > trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java, line 48
> > <https://reviews.apache.org/r/4458/diff/2/?file=95128#file95128line48>
> >
> > i don't think this case is ever true... this checks that the 2 references are equal (i.e. pointing to the same object)... perhaps you meant: m1.equals(m2)... however if m1 is null, then NullPointerException will be thrown here
actually... my bad here... this checks the null == null case i asked for above... good idea!
- brian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/#review6368
-----------------------------------------------------------
On 2012-03-23 21:19:16, Ricky Nguyen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4458/
> -----------------------------------------------------------
>
> (Updated 2012-03-23 21:19:16)
>
>
> Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
>
>
> Summary
> -------
>
> putting nulls last
>
>
> This addresses bug OODT-429.
> https://issues.apache.org/jira/browse/OODT-429
>
>
> Diffs
> -----
>
> trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
> trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4458/diff
>
>
> Testing
> -------
>
> wrote unit test
>
>
> Thanks,
>
> Ricky
>
>
Re: Review Request: filemgr query throws NPE when sorting by key with
possibly empty values
Posted by brian Foster <ho...@juno.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/#review6368
-----------------------------------------------------------
trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java
<https://reviews.apache.org/r/4458/#comment13827>
i don't think this case is ever true... this checks that the 2 references are equal (i.e. pointing to the same object)... perhaps you meant: m1.equals(m2)... however if m1 is null, then NullPointerException will be thrown here
trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java
<https://reviews.apache.org/r/4458/#comment13829>
nice test... thanks!
- brian
On 2012-03-23 21:19:16, Ricky Nguyen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4458/
> -----------------------------------------------------------
>
> (Updated 2012-03-23 21:19:16)
>
>
> Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
>
>
> Summary
> -------
>
> putting nulls last
>
>
> This addresses bug OODT-429.
> https://issues.apache.org/jira/browse/OODT-429
>
>
> Diffs
> -----
>
> trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
> trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4458/diff
>
>
> Testing
> -------
>
> wrote unit test
>
>
> Thanks,
>
> Ricky
>
>
Re: Review Request: filemgr query throws NPE when sorting by key with
possibly empty values
Posted by Ricky Nguyen <ri...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/
-----------------------------------------------------------
(Updated 2012-03-23 21:19:16.059506)
Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
Changes
-------
test with multiple nulls. since order of null items is indeterminate, take sublist of null items, then test size and test contains.
Summary
-------
putting nulls last
This addresses bug OODT-429.
https://issues.apache.org/jira/browse/OODT-429
Diffs (updated)
-----
trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
Diff: https://reviews.apache.org/r/4458/diff
Testing
-------
wrote unit test
Thanks,
Ricky
Re: Review Request: filemgr query throws NPE when sorting by key with
possibly empty values
Posted by brian Foster <ho...@juno.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4458/#review6282
-----------------------------------------------------------
Great patch!.... Couple comments
trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java
<https://reviews.apache.org/r/4458/#comment13625>
Should handle case where both are null... will produce faster sorting time in cases where there are several nulls... something along the lines of:
if (m1 == null) {
If (m2 == null) {
return 0;
}
return 1;
}
if (m2 == null) {
return -1;
}
trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java
<https://reviews.apache.org/r/4458/#comment13626>
Maybe add test case with multiple nulls?
- brian
On 2012-03-23 00:10:25, Ricky Nguyen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4458/
> -----------------------------------------------------------
>
> (Updated 2012-03-23 00:10:25)
>
>
> Review request for oodt, Chris Mattmann, brian Foster, Paul Ramirez, Sheryl John, and Thomas Bennett.
>
>
> Summary
> -------
>
> putting nulls last
>
>
> This addresses bug OODT-429.
> https://issues.apache.org/jira/browse/OODT-429
>
>
> Diffs
> -----
>
> trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/structs/query/QueryResultComparator.java 1303205
> trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/structs/query/TestQueryResultComparator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4458/diff
>
>
> Testing
> -------
>
> wrote unit test
>
>
> Thanks,
>
> Ricky
>
>