Bug: Not-Null Values are at top when fetching list from database, in Release 9.04

[Divesh Dutta]

Hello Developers,

I see an strange issue on Release 9.04. But that issue does not exists
on latest OFBiz trunk. Below is brief description  of  issue:

1) When I use  any of the method (like findList or findByAnd) of
DelegatorImpl.java class , and sort it by "sequenceNum", For eg in
EditProductFeatures.groovy: (Release 9.04)

    context.productFeatureAndAppls =
delegator.findList('ProductFeatureAndAppl',
        EntityCondition.makeCondition([productId : productId]), null,
        ['sequenceNum', 'productFeatureApplTypeId',
'productFeatureTypeId', 'description'], null, false);

It returns me the list of values, with *Not-Null values at the top* ,
and then it sort in Ascending order by "sequenceNum", ......

2) But When I use Latest trunk in OFBiz: Using same example, It returns
me the list sorted by "sequenceNum", .... in ascending order and then
*Not-null values at the bottom

*3) I think this is the major bug in Release 9.04, because if we think
at application level, if a catagory has over 800 products, Catalog
Manager will have to go to the last page, to sequence every single
product for it to show properly on the front end.

4) Instead if Catalog Manager want to sequence the products,  he will  
arrange them at very first page.

5) I tried to found the reason of this major difference, but could not
locate the exact fix in any of the commit. So I request all the
developers, if any one have any idea regarding this please share your
views here. Also I think this should be fixed in Release 9.04 as well.

Thanks
--
Divesh Dutta.

[David E. Jones-2]

For that call the sorting is done in the database (not cached), so there's probably a difference in databases or database configs.

-David


On Apr 23, 2010, at 8:49 AM, Divesh Dutta wrote:

> Hello Developers,
>
> I see an strange issue on Release 9.04. But that issue does not exists on latest OFBiz trunk. Below is brief description  of  issue:
>
> 1) When I use  any of the method (like findList or findByAnd) of DelegatorImpl.java class , and sort it by "sequenceNum", For eg in EditProductFeatures.groovy: (Release 9.04)
>
>   context.productFeatureAndAppls = delegator.findList('ProductFeatureAndAppl',
>       EntityCondition.makeCondition([productId : productId]), null,
>       ['sequenceNum', 'productFeatureApplTypeId', 'productFeatureTypeId', 'description'], null, false);
>
> It returns me the list of values, with *Not-Null values at the top* , and then it sort in Ascending order by "sequenceNum", ......
>
> 2) But When I use Latest trunk in OFBiz: Using same example, It returns me the list sorted by "sequenceNum", .... in ascending order and then *Not-null values at the bottom
>
> *3) I think this is the major bug in Release 9.04, because if we think at application level, if a catagory has over 800 products, Catalog Manager will have to go to the last page, to sequence every single product for it to show properly on the front end.
>
> 4) Instead if Catalog Manager want to sequence the products,  he will  arrange them at very first page.
>
> 5) I tried to found the reason of this major difference, but could not locate the exact fix in any of the commit. So I request all the developers, if any one have any idea regarding this please share your views here. Also I think this should be fixed in Release 9.04 as well.
>
> Thanks
> --
> Divesh Dutta.
>

[Adam Heath-2]

David E Jones wrote:
> For that call the sorting is done in the database (not cached), so there's probably a difference in databases or database configs.

Not entirely accurate.  The first match on a condition/entity is
cached, as it is returned from the database.  If a later call is only
different on the ordering, then the system just reorders in memory
from the previously cached query.

> On Apr 23, 2010, at 8:49 AM, Divesh Dutta wrote:
>
>> Hello Developers,
>>
>> I see an strange issue on Release 9.04. But that issue does not exists on latest OFBiz trunk. Below is brief description  of  issue:
>>
>> 1) When I use  any of the method (like findList or findByAnd) of DelegatorImpl.java class , and sort it by "sequenceNum", For eg in EditProductFeatures.groovy: (Release 9.04)
>>
>>   context.productFeatureAndAppls = delegator.findList('ProductFeatureAndAppl',
>>       EntityCondition.makeCondition([productId : productId]), null,
>>       ['sequenceNum', 'productFeatureApplTypeId', 'productFeatureTypeId', 'description'], null, false);
>>
>> It returns me the list of values, with *Not-Null values at the top* , and then it sort in Ascending order by "sequenceNum", ......
>>
>> 2) But When I use Latest trunk in OFBiz: Using same example, It returns me the list sorted by "sequenceNum", .... in ascending order and then *Not-null values at the bottom
>>
>> *3) I think this is the major bug in Release 9.04, because if we think at application level, if a catagory has over 800 products, Catalog Manager will have to go to the last page, to sequence every single product for it to show properly on the front end.
>>
>> 4) Instead if Catalog Manager want to sequence the products,  he will  arrange them at very first page.
>>
>> 5) I tried to found the reason of this major difference, but could not locate the exact fix in any of the commit. So I request all the developers, if any one have any idea regarding this please share your views here. Also I think this should be fixed in Release 9.04 as well.
>>
>> Thanks
>> --
>> Divesh Dutta.
>>
>

[David E. Jones-2]

On Apr 23, 2010, at 10:37 AM, Adam Heath wrote:

> David E Jones wrote:
>> For that call the sorting is done in the database (not cached), so there's probably a difference in databases or database configs.
>
> Not entirely accurate.  The first match on a condition/entity is
> cached, as it is returned from the database.  If a later call is only
> different on the ordering, then the system just reorders in memory
> from the previously cached query.

I don't understand your reply or how it applies to what I wrote. I didn't write anything about how sorting in the cache worked, just that it wasn't relevant because the call below was not cached. Could you explain?

-David

>
>> On Apr 23, 2010, at 8:49 AM, Divesh Dutta wrote:
>>
>>> Hello Developers,
>>>
>>> I see an strange issue on Release 9.04. But that issue does not exists on latest OFBiz trunk. Below is brief description  of  issue:
>>>
>>> 1) When I use  any of the method (like findList or findByAnd) of DelegatorImpl.java class , and sort it by "sequenceNum", For eg in EditProductFeatures.groovy: (Release 9.04)
>>>
>>>  context.productFeatureAndAppls = delegator.findList('ProductFeatureAndAppl',
>>>      EntityCondition.makeCondition([productId : productId]), null,
>>>      ['sequenceNum', 'productFeatureApplTypeId', 'productFeatureTypeId', 'description'], null, false);
>>>
>>> It returns me the list of values, with *Not-Null values at the top* , and then it sort in Ascending order by "sequenceNum", ......
>>>
>>> 2) But When I use Latest trunk in OFBiz: Using same example, It returns me the list sorted by "sequenceNum", .... in ascending order and then *Not-null values at the bottom
>>>
>>> *3) I think this is the major bug in Release 9.04, because if we think at application level, if a catagory has over 800 products, Catalog Manager will have to go to the last page, to sequence every single product for it to show properly on the front end.
>>>
>>> 4) Instead if Catalog Manager want to sequence the products,  he will  arrange them at very first page.
>>>
>>> 5) I tried to found the reason of this major difference, but could not locate the exact fix in any of the commit. So I request all the developers, if any one have any idea regarding this please share your views here. Also I think this should be fixed in Release 9.04 as well.
>>>
>>> Thanks
>>> --
>>> Divesh Dutta.
>>>
>>
>

[Adam Heath-2]

David E Jones wrote:

> On Apr 23, 2010, at 10:37 AM, Adam Heath wrote:
>
>> David E Jones wrote:
>>> For that call the sorting is done in the database (not cached), so there's probably a difference in databases or database configs.
>> Not entirely accurate.  The first match on a condition/entity is
>> cached, as it is returned from the database.  If a later call is only
>> different on the ordering, then the system just reorders in memory
>> from the previously cached query.
>
> I don't understand your reply or how it applies to what I wrote. I didn't write anything about how sorting in the cache worked, just that it wasn't relevant because the call below was not cached. Could you explain?

Maybe this use case below is hitting the in-memory sorting, because
some other part of the code is running the same query, but with a
different ordering key.  So the earlier query is database sorted, but
the bug reported below is running a second, identical query, but with
a different ordering. This could happen if you click a column sort.

I'm speaking more general-purpose.  Based on your comment, I could see
databases having different handling of null values in sorts.  So
ideally, the cache system should handle that case.

And, now that I see it, you're right, the code below is not cached.
So, everything I just wrote doesn't apply to *this* issue.  But it
could be some other issue waiting to bite us.

>
> -David
>
>
>>> On Apr 23, 2010, at 8:49 AM, Divesh Dutta wrote:
>>>
>>>> Hello Developers,
>>>>
>>>> I see an strange issue on Release 9.04. But that issue does not exists on latest OFBiz trunk. Below is brief description  of  issue:
>>>>
>>>> 1) When I use  any of the method (like findList or findByAnd) of DelegatorImpl.java class , and sort it by "sequenceNum", For eg in EditProductFeatures.groovy: (Release 9.04)
>>>>
>>>>  context.productFeatureAndAppls = delegator.findList('ProductFeatureAndAppl',
>>>>      EntityCondition.makeCondition([productId : productId]), null,
>>>>      ['sequenceNum', 'productFeatureApplTypeId', 'productFeatureTypeId', 'description'], null, false);
>>>>
>>>> It returns me the list of values, with *Not-Null values at the top* , and then it sort in Ascending order by "sequenceNum", ......
>>>>
>>>> 2) But When I use Latest trunk in OFBiz: Using same example, It returns me the list sorted by "sequenceNum", .... in ascending order and then *Not-null values at the bottom
>>>>
>>>> *3) I think this is the major bug in Release 9.04, because if we think at application level, if a catagory has over 800 products, Catalog Manager will have to go to the last page, to sequence every single product for it to show properly on the front end.
>>>>
>>>> 4) Instead if Catalog Manager want to sequence the products,  he will  arrange them at very first page.
>>>>
>>>> 5) I tried to found the reason of this major difference, but could not locate the exact fix in any of the commit. So I request all the developers, if any one have any idea regarding this please share your views here. Also I think this should be fixed in Release 9.04 as well.
>>>>
>>>> Thanks
>>>> --
>>>> Divesh Dutta.
>>>>
>

[Bob Morley]

There are differences in how the various databases elect to sort nulls  
using a standard order by clause.  Some databases (Postgres and Oracle  
I believe) support a "NULLS FIRST" or "NULLS LAST" grammar on the  
order by clause to dictate this.

I meant to package this up as a patch, but in our solution we provided  
consistent ordering regardless of database as well as the ability on  
an order-by element to indicate if you want the nulls ordered first or  
last for that particular field.  There is also a new element in the  
entityengine.xml that indicates if a particular data source supports  
the "nulls-first" grammar.

If there is interest and a committer that would be willing to reivew,  
I can package this up ASAP and you can consider for back porting.

On Apr 23, 2010, at 11:18 AM, David E Jones wrote:

>
> For that call the sorting is done in the database (not cached), so  
> there's probably a difference in databases or database configs.
>
> -David
>
>
> On Apr 23, 2010, at 8:49 AM, Divesh Dutta wrote:
>
>> Hello Developers,
>>
>> I see an strange issue on Release 9.04. But that issue does not  
>> exists on latest OFBiz trunk. Below is brief description  of  issue:
>>
>> 1) When I use  any of the method (like findList or findByAnd) of  
>> DelegatorImpl.java class , and sort it by "sequenceNum", For eg in  
>> EditProductFeatures.groovy: (Release 9.04)
>>
>>  context.productFeatureAndAppls =  
>> delegator.findList('ProductFeatureAndAppl',
>>      EntityCondition.makeCondition([productId : productId]), null,
>>      ['sequenceNum', 'productFeatureApplTypeId',  
>> 'productFeatureTypeId', 'description'], null, false);
>>
>> It returns me the list of values, with *Not-Null values at the  
>> top* , and then it sort in Ascending order by "sequenceNum", ......
>>
>> 2) But When I use Latest trunk in OFBiz: Using same example, It  
>> returns me the list sorted by "sequenceNum", .... in ascending  
>> order and then *Not-null values at the bottom
>>
>> *3) I think this is the major bug in Release 9.04, because if we  
>> think at application level, if a catagory has over 800 products,  
>> Catalog Manager will have to go to the last page, to sequence every  
>> single product for it to show properly on the front end.
>>
>> 4) Instead if Catalog Manager want to sequence the products,  he  
>> will  arrange them at very first page.
>>
>> 5) I tried to found the reason of this major difference, but could  
>> not locate the exact fix in any of the commit. So I request all the  
>> developers, if any one have any idea regarding this please share  
>> your views here. Also I think this should be fixed in Release 9.04  
>> as well.
>>
>> Thanks
>> --
>> Divesh Dutta.
>>
>

Robert Morley
Senior Software Developer
Emforium Group Inc.
ALL-IN Software™
519-772-6824 ext 220
[hidden email]

[Divesh Dutta]

Hello Robert,

To be more clear, I am using same database and database drivers on both
the instances. By the way I think  "New element in the entityengine.xml
that indicates if a particular data source supports the nulls-first
grammar" will be good idea to add on.

Can you please create a jira issue and load your patch, so that
Committers can have a look and give their views on this.

Thanks
--
Divesh Dutta.


Robert Morley wrote:

> There are differences in how the various databases elect to sort nulls
> using a standard order by clause.  Some databases (Postgres and Oracle
> I believe) support a "NULLS FIRST" or "NULLS LAST" grammar on the
> order by clause to dictate this.
>
> I meant to package this up as a patch, but in our solution we provided
> consistent ordering regardless of database as well as the ability on
> an order-by element to indicate if you want the nulls ordered first or
> last for that particular field.  There is also a new element in the
> entityengine.xml that indicates if a particular data source supports
> the "nulls-first" grammar.
>
> If there is interest and a committer that would be willing to reivew,
> I can package this up ASAP and you can consider for back porting.
>
> On Apr 23, 2010, at 11:18 AM, David E Jones wrote:
>
>>
>> For that call the sorting is done in the database (not cached), so
>> there's probably a difference in databases or database configs.
>>
>> -David
>>
>>
>> On Apr 23, 2010, at 8:49 AM, Divesh Dutta wrote:
>>
>>> Hello Developers,
>>>
>>> I see an strange issue on Release 9.04. But that issue does not
>>> exists on latest OFBiz trunk. Below is brief description  of  issue:
>>>
>>> 1) When I use  any of the method (like findList or findByAnd) of
>>> DelegatorImpl.java class , and sort it by "sequenceNum", For eg in
>>> EditProductFeatures.groovy: (Release 9.04)
>>>
>>>  context.productFeatureAndAppls =
>>> delegator.findList('ProductFeatureAndAppl',
>>>      EntityCondition.makeCondition([productId : productId]), null,
>>>      ['sequenceNum', 'productFeatureApplTypeId',
>>> 'productFeatureTypeId', 'description'], null, false);
>>>
>>> It returns me the list of values, with *Not-Null values at the top*
>>> , and then it sort in Ascending order by "sequenceNum", ......
>>>
>>> 2) But When I use Latest trunk in OFBiz: Using same example, It
>>> returns me the list sorted by "sequenceNum", .... in ascending order
>>> and then *Not-null values at the bottom
>>>
>>> *3) I think this is the major bug in Release 9.04, because if we
>>> think at application level, if a catagory has over 800 products,
>>> Catalog Manager will have to go to the last page, to sequence every
>>> single product for it to show properly on the front end.
>>>
>>> 4) Instead if Catalog Manager want to sequence the products,  he
>>> will  arrange them at very first page.
>>>
>>> 5) I tried to found the reason of this major difference, but could
>>> not locate the exact fix in any of the commit. So I request all the
>>> developers, if any one have any idea regarding this please share
>>> your views here. Also I think this should be fixed in Release 9.04
>>> as well.
>>>
>>> Thanks
>>> --
>>> Divesh Dutta.
>>>
>>
>
> Robert Morley
> Senior Software Developer
> Emforium Group Inc.
> ALL-IN Software™
> 519-772-6824 ext 220
> [hidden email]
>
>

[Bob Morley]

No problemo; I will try to get that patch up in the next 48 hours.

It is very odd that you are seeing different results with the same database / drivers.  My understanding is that the sql standard does not dictate how nulls are sorted and it seemed a split as to having them first or last based on the DBMS in question.

At any rate, I will post the JIRA ticket back into this thread when a patch is available.

- Bob

----- Original Message -----
From: "Divesh Dutta" <[hidden email]>
To: [hidden email]
Sent: Saturday, April 24, 2010 1:18:44 AM
Subject: Re: Bug: Not-Null Values are at top when fetching list from database, in Release 9.04

Hello Robert,

To be more clear, I am using same database and database drivers on both
the instances. By the way I think  "New element in the entityengine.xml
that indicates if a particular data source supports the nulls-first
grammar" will be good idea to add on.

Can you please create a jira issue and load your patch, so that
Committers can have a look and give their views on this.

[Bob Morley]

Bob Morley wrote

Can you please create a jira issue and load your patch, so that
Committers can have a look and give their views on this.

Ok I have created a patch -> https://issues.apache.org/jira/browse/OFBIZ-3740.  We can continue any discussion on that patch; but it should be noted that this patch will not do anything unless an order-by is updated to include "NULLS FIRST" or "NULLS LAST".

[Jacques Le Roux]

Hi Divesh,

Would be interested by testing this patch?

Thanks

Jacques

From: "Bob Morley" <[hidden email]>

> Bob Morley wrote:
>>
>> Can you please create a jira issue and load your patch, so that
>> Committers can have a look and give their views on this.
>>
>
> Ok I have created a patch ->
> https://issues.apache.org/jira/browse/OFBIZ-3740.  We can continue any
> discussion on that patch; but it should be noted that this patch will not do
> anything unless an order-by is updated to include "NULLS FIRST" or "NULLS
> LAST".
> --
> View this message in context:
> http://ofbiz.135035.n4.nabble.com/Bug-Not-Null-Values-are-at-top-when-fetching-list-from-database-in-Release-9-04-tp2062221p2064444.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>

[Divesh Dutta]

Hi Jacques and Bob,

Sorry to reply late on this. I tested this patch and found that this
feature is only supported if we use the derby, postgres, and oracle
databases. This is not supported for Mysql database.
So what I think is this is not a generic solution. But yes it works as
expected, for derby, postgres, and oracle databases. Also I have put my
comments on https://issues.apache.org/jira/browse/OFBIZ-3740

If we can provide a generic solution, this will be a good add-on. Please
share your views.

Thanks
--
Divesh Dutta.


Jacques Le Roux wrote:

> Hi Divesh,
>
> Would be interested by testing this patch?
>
> Thanks
>
> Jacques
>
> From: "Bob Morley" <[hidden email]>
>> Bob Morley wrote:
>>>
>>> Can you please create a jira issue and load your patch, so that
>>> Committers can have a look and give their views on this.
>>>
>>
>> Ok I have created a patch ->
>> https://issues.apache.org/jira/browse/OFBIZ-3740.  We can continue any
>> discussion on that patch; but it should be noted that this patch will
>> not do
>> anything unless an order-by is updated to include "NULLS FIRST" or
>> "NULLS
>> LAST".
>> --
>> View this message in context:
>> http://ofbiz.135035.n4.nabble.com/Bug-Not-Null-Values-are-at-top-when-fetching-list-from-database-in-Release-9-04-tp2062221p2064444.html 
>>
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>
>
>

[Adam Heath-2]

Divesh Dutta wrote:

> Hi Jacques and Bob,
>
> Sorry to reply late on this. I tested this patch and found that this
> feature is only supported if we use the derby, postgres, and oracle
> databases. This is not supported for Mysql database.
> So what I think is this is not a generic solution. But yes it works as
> expected, for derby, postgres, and oracle databases. Also I have put my
> comments on https://issues.apache.org/jira/browse/OFBIZ-3740
>
> If we can provide a generic solution, this will be a good add-on. Please
> share your views.

Is it possible that some jdbc metadata parameter can specify whether
the database supports this?

[Bob Morley]

Adam Heath-2 wrote

> If we can provide a generic solution, this will be a good add-on. Please
> share your views.

Is it possible that some jdbc metadata parameter can specify whether
the database supports this?

The patch contains a parameter:

+        use-order-by-nulls="true">

for this purpose.  For the databases listed it will generate a database native "NULLS FIRST/LAST" statement as part of the order by clause.  For the other databases (the ones where this property is not set or is set to "false") it will generate a case:

+        if ((nullsFirst != null) && (!datasourceInfo.useOrderByNulls)) {
+            sb.append("CASE WHEN ");
+            getValue().addSqlValue(sb, modelEntity, null, includeTablenamePrefix, datasourceInfo);
+            sb.append(" IS NULL THEN ");
+            sb.append(nullsFirst ? "0" : "1");
+            sb.append(" ELSE ");
+            sb.append(nullsFirst ? "1" : "0");
+            sb.append(" END, ");
+        }
+        

It should be noted that this generation only occurs if the "nulls" is explicitly specified on the order-by clause.  I thought about always generating this snippet to support a "default sorting behaviour" across all databases, but I did not want to alter existing sql statements without a discussion on that.  So with this patch one can explicitly specify how they want nulls sorted and it should work for all databases.