svn commit: r950901 - in /ofbiz/trunk/framework/entity: config/entityengine.xml dtd/entity-config.xsd src/org/ofbiz/entity/condition/OrderByItem.java src/org/ofbiz/entity/config/DatasourceInfo.java

Author: jleroux
Date: Thu Jun  3 07:39:19 2010
New Revision: 950901

URL: http://svn.apache.org/viewvc?rev=950901&view=rev
Log:
A patch from Bob Morley "Support for order by nulls first/last " (https://issues.apache.org/jira/browse/OFBIZ-3740) - OFBIZ-3740

This patch provides the fundamentals to resolve an issue where the sorting of null values were inconsistent (sometimes sorting first and other times sorting last). Specifically, this patch allows one to create an order-by clause via any of the order-by elements of "-myField NULLS LAST" where this text parses out the - (as descending) and the "nulls last".

Since not all databases support the "nulls" argument on an order by clause (it was introduced as part of the OLAP support specification) , our entity-engine.xml file allows each data source to indicate if it has this support. If the support does not exist and nulls first/last is specified, the sql that is generated uses native sql to simulate the nulls first/last intent. At this time, the derby, postgres, and oracle databases are marked to use the nulls first grammar.

Right now, if you do not specify "NULLS XXX" in the field-name for the order-by it makes no change whatsever (naturally this assumption could be changed to have a default).

It should be noted, that my intent here was ultimately to "properly" model the order by into something whose xml representation could look something like ...
<entity-order-by field-name="fieldName" ascending="true" nulls-first="true" />

Divesh Dutta added a comment - 07/May/10 01:44 AM HI Bob, This patch works as expected for derby, postgres, and oracle databases as you have already mentioned. Cant we have some generic solution which supports all the databases. Or we can have some conditional code, where we can check which datasource is being used, and then generate the sql accordingly. Please let me know your views on this.

Bob Morley added a comment - 07/May/10 08:07 AM
Hi Divesh,
I added a more detailed description in the forum thread; but in brief this is a generic solution which supports all databases which generates the sql accordingly. What this patch does not do is apply a standard default order for all databases; it just adds support for one to dictate how they want the nulls sorted.
For example, when one sets the order-by they can set it to "lastName DESC NULLS FIRST" which should apply to all databases.


The thread Bob was referring above ends at http://markmail.org/message/47z7fwtawehuem6b

Modified:
    ofbiz/trunk/framework/entity/config/entityengine.xml
    ofbiz/trunk/framework/entity/dtd/entity-config.xsd
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/OrderByItem.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/config/DatasourceInfo.java

Modified: ofbiz/trunk/framework/entity/config/entityengine.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/config/entityengine.xml?rev=950901&r1=950900&r2=950901&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/config/entityengine.xml (original)
+++ ofbiz/trunk/framework/entity/config/entityengine.xml Thu Jun  3 07:39:19 2010
@@ -158,7 +158,8 @@ access. For a detailed description see t
             add-missing-on-start="true"
             use-pk-constraint-names="false"
             use-indices-unique="false"
-            alias-view-columns="false">
+            alias-view-columns="false"
+            use-order-by-nulls="true">
         <read-data reader-name="seed"/>
         <read-data reader-name="seed-initial"/>
         <read-data reader-name="demo"/>
@@ -182,7 +183,8 @@ access. For a detailed description see t
         add-missing-on-start="true"
         use-pk-constraint-names="false"
         use-indices-unique="false"
-        alias-view-columns="false">
+        alias-view-columns="false"
+        use-order-by-nulls="true">
         <inline-jdbc
             jdbc-driver="org.apache.derby.jdbc.EmbeddedDriver"
             jdbc-uri="jdbc:derby:ofbizodbc;create=true"
@@ -202,7 +204,8 @@ access. For a detailed description see t
             add-missing-on-start="true"
             use-pk-constraint-names="false"
             use-indices-unique="false"
-            alias-view-columns="false">
+            alias-view-columns="false"
+            use-order-by-nulls="true">
         <read-data reader-name="seed"/>
         <read-data reader-name="seed-initial"/>
         <read-data reader-name="demo"/>
@@ -226,7 +229,8 @@ access. For a detailed description see t
         add-missing-on-start="true"
         use-pk-constraint-names="false"
         use-indices-unique="false"
-        alias-view-columns="false">
+        alias-view-columns="false"
+        use-order-by-nulls="true">
         <read-data reader-name="seed"/>
         <read-data reader-name="seed-initial"/>
         <read-data reader-name="demo"/>
@@ -373,7 +377,8 @@ access. For a detailed description see t
             use-fk-initially-deferred="false"
             alias-view-columns="false"
             join-style="ansi"
-            use-binary-type-for-blob="true">
+            use-binary-type-for-blob="true"
+            use-order-by-nulls="true">
             <!-- use this attribute to make the EntityListIterator more effective for pgjdbc 7.5devel and later:
                 result-fetch-size="50"
             -->
@@ -412,7 +417,8 @@ access. For a detailed description see t
             alias-view-columns="false"
             join-style="ansi"
             result-fetch-size="50"
-            use-binary-type-for-blob="true">
+            use-binary-type-for-blob="true"
+            use-order-by-nulls="true">
         <read-data reader-name="seed"/>
         <read-data reader-name="seed-initial"/>
         <read-data reader-name="demo"/>
@@ -435,7 +441,8 @@ access. For a detailed description see t
             check-on-start="true"
             add-missing-on-start="true"
             alias-view-columns="false"
-            join-style="ansi">
+            join-style="ansi"
+            use-order-by-nulls="true">
         <read-data reader-name="seed"/>
         <read-data reader-name="seed-initial"/>
         <read-data reader-name="demo"/>
@@ -455,7 +462,8 @@ access. For a detailed description see t
             field-type-name="oracle"
             check-on-start="true"
             add-missing-on-start="true"
-            join-style="ansi">
+            join-style="ansi"
+            use-order-by-nulls="true">
         <read-data reader-name="main"/>
         <inline-jdbc
                 jdbc-driver="com.ddtek.jdbc.oracle.OracleDriver"

Modified: ofbiz/trunk/framework/entity/dtd/entity-config.xsd
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/dtd/entity-config.xsd?rev=950901&r1=950900&r2=950901&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/dtd/entity-config.xsd (original)
+++ ofbiz/trunk/framework/entity/dtd/entity-config.xsd Thu Jun  3 07:39:19 2010
@@ -381,6 +381,14 @@ under the License.
                 </xs:restriction>
             </xs:simpleType>
         </xs:attribute>
+        <xs:attribute name="use-order-by-nulls" default="false">
+            <xs:simpleType>
+                <xs:restriction base="xs:token">
+                    <xs:enumeration value="true"/>
+                    <xs:enumeration value="false"/>
+                </xs:restriction>
+            </xs:simpleType>
+        </xs:attribute>
         <xs:attribute type="xs:string" name="table-type"/>
         <xs:attribute type="xs:string" name="character-set"/>
         <xs:attribute type="xs:string" name="collate"/>

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/OrderByItem.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/OrderByItem.java?rev=950901&r1=950900&r2=950901&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/OrderByItem.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/OrderByItem.java Thu Jun  3 07:39:19 2010
@@ -32,7 +32,11 @@ public class OrderByItem implements Comp
     public static final int UPPER   = 1;
     public static final int LOWER   = 2;
 
+    public static final String NULLS_FIRST = "NULLS FIRST";
+    public static final String NULLS_LAST = "NULLS LAST";
+
     protected boolean descending;
+    protected Boolean nullsFirst;
     protected EntityConditionValue value;
 
     public OrderByItem(EntityConditionValue value) {
@@ -44,6 +48,11 @@ public class OrderByItem implements Comp
         this.descending = descending;
     }
 
+    public OrderByItem(EntityConditionValue value, boolean descending, Boolean nullsFirst) {
+        this(value, descending);
+        this.nullsFirst = nullsFirst;
+    }
+
     public EntityConditionValue getValue() {
         return value;
     }
@@ -66,6 +75,19 @@ public class OrderByItem implements Comp
 
     public static final OrderByItem parse(String text) {
         text = text.trim();
+        
+        // handle nulls first/last
+        Boolean nullsFirst = null;
+        if (text.toUpperCase().endsWith(NULLS_FIRST)) {
+            nullsFirst = true;
+            text = text.substring(0, text.length() - NULLS_FIRST.length()).trim();
+        }
+        
+        if (text.toUpperCase().endsWith(NULLS_LAST)) {
+            nullsFirst = false;
+            text = text.substring(0, text.length() - NULLS_LAST.length()).trim();
+        }
+
         int startIndex = 0, endIndex = text.length();
         boolean descending;
         int caseSensitivity;
@@ -121,7 +143,7 @@ public class OrderByItem implements Comp
                 value = EntityFunction.LOWER(value);
                 break;
         }
-        return new OrderByItem(value, descending);
+        return new OrderByItem(value, descending, nullsFirst);
     }
 
     public void checkOrderBy(ModelEntity modelEntity) throws GenericModelException {
@@ -152,8 +174,22 @@ public class OrderByItem implements Comp
     }
 
     public void makeOrderByString(StringBuilder sb, ModelEntity modelEntity, boolean includeTablenamePrefix, DatasourceInfo datasourceInfo) {
+        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, ");
+        }
+        
         getValue().addSqlValue(sb, modelEntity, null, includeTablenamePrefix, datasourceInfo);
         sb.append(descending ? " DESC" : " ASC");
+        
+        if ((nullsFirst != null) && (datasourceInfo.useOrderByNulls)) {
+            sb.append(nullsFirst ? " NULLS FIRST" : " NULLS LAST");
+        }
     }
 
     @Override
@@ -169,6 +205,9 @@ public class OrderByItem implements Comp
         StringBuilder sb = new StringBuilder();
         sb.append(getValue());
         sb.append(descending ? " DESC" : " ASC");
+        if (nullsFirst != null) {
+            sb.append(nullsFirst ? " NULLS FIRST" : " NULLS LAST");
+        }
         return sb.toString();
     }
 }

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/config/DatasourceInfo.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/config/DatasourceInfo.java?rev=950901&r1=950900&r2=950901&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/config/DatasourceInfo.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/config/DatasourceInfo.java Thu Jun  3 07:39:19 2010
@@ -72,6 +72,7 @@ public class DatasourceInfo {
     public boolean alwaysUseConstraintKeyword = false;
     public boolean dropFkUseForeignKeyKeyword = false;
     public boolean useBinaryTypeForBlob = false;
+    public boolean useOrderByNulls = false;
     public String tableType = null;
     public String characterSet = null;
     public String collate = null;
@@ -155,6 +156,7 @@ public class DatasourceInfo {
             this.alwaysUseConstraintKeyword = "true".equals(datasourceElement.getAttribute("always-use-constraint-keyword"));
             this.dropFkUseForeignKeyKeyword = "true".equals(datasourceElement.getAttribute("drop-fk-use-foreign-key-keyword"));
             this.useBinaryTypeForBlob = "true".equals(datasourceElement.getAttribute("use-binary-type-for-blob"));
+            this.useOrderByNulls = "true".equals(datasourceElement.getAttribute("use-order-by-nulls"));
 
             this.tableType = datasourceElement.getAttribute("table-type");
             this.characterSet = datasourceElement.getAttribute("character-set");