Issue Details (XML | Word | Printable)

Key: NUCRDBMS-758
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: Alexander Ley
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
DataNucleus Store RDBMS

Wrong SQL generated for bulk fetch for queries with order by

Created: 30/Jan/14 06:20 PM   Updated: 01/Mar/14 07:39 PM   Resolved: 21/Feb/14 03:24 PM
Component/s: Datastore Adapter
Affects Version/s: 3.2.11
Fix Version/s: 3.2.12, 4.0.0.m1

Datastore: Oracle


 Description  « Hide
"SELECT FROM mydomain.model.Master ORDER BY id" with a fetch group "details" generates a bulk fetch SQL like
SELECT 'mydomain.model.Detail' AS NUCLEUS_TYPE,A0.ID,A0."NAME",A0.MASTER_ID_OID,A0.DETAILS_INTEGER_IDX AS NUCORDER0
FROM DETAIL A0
WHERE A0.DETAILS_INTEGER_IDX >= 0 AND EXISTS
(
 SELECT 'mydomain.model.Master' AS NUCLEUS_TYPE,A0_SUB.ID AS DN_APPID,A0_SUB.ID AS NUCORDER0
 FROM MASTER A0_SUB
 WHERE A0.MASTER_ID_OID = A0_SUB.ID
 ORDER BY NUCORDER0
)
ORDER BY NUCORDER0
Oracle does not allow for "ORDER BY" in an exists subclause. I do not know about other databases.
Th ORDER BY from the master query has to be removed for the exists subquery.
Test case in https://github.com/aley2003/test-jdo

Alexander Ley added a comment - 19/Feb/14 06:07 PM
Further investigations showed the following problems with bulk fetch selects:

A JDO query like "select from mydomain.model.Master WHERE id>=1 ORDER BY id"

gets a bulk fetch SELECT compiled to

SELECT 'mydomain.model.Detail' AS NUCLEUS_TYPE,A0.ID,A0."NAME",
   A0.MASTER_ID_OID, A0.DETAILS_INTEGER_IDX AS NUCORDER0
FROM DETAIL A0
WHERE A0.DETAILS_INTEGER_IDX >= 0
AND EXISTS (
  *** SELECT of columns useless, one column (e.g. constant 1) is enough: ***
   SELECT 'mydomain.model.Master' AS NUCLEUS_TYPE,A0_SUB.ID AS DN_APPID,A0_SUB.ID AS NUCORDER0
   FROM MASTER A0_SUB
   WHERE A0_SUB.ID >= 1
   AND A0.MASTER_ID_OID = A0_SUB.ID
  *** ORDER BY is useless and forbitten in Oracle for EXISTS subquery: ***
   ORDER BY NUCORDER0
)
*** ORDER BY on DETAILS_INTEGER_IDX does not make any sense for the bulk fetch.
  Perhaps it should be ORDER BY MASTER_ID_OID, DETAILS_INTEGER_IDX? ***
ORDER BY NUCORDER0

I have tried to find the correct place to work on the problem but got lost in all the different Queries and Statements.

Andy Jefferson added a comment - 20/Feb/14 05:50 PM
Any change to Bulk Fetch would only affect code in BulkFetchHelper, and it isn't large. If the ordering of a SQLStatement is unimportant (like in EXISTS) then call sqlStmt.setOrdering(null) or something similar and that should remove the ordering (before the statement SQL is generated). When you have that working, attach it here, or provide a GitHub pull request.

Datastores I use don't have a problem with ordering in EXISTS (as redundant as it is)

Alexander Ley added a comment - 21/Feb/14 02:43 PM
I have inserted the setOrdering() in BulkFetchHelper and it seems to work.
Here is the Diff:
ac5fa195dd679070644e4b5a73ce4a81040b07da
 src/java/org/datanucleus/store/rdbms/query/BulkFetchHelper.java | 5 ++++-
 src/java/org/datanucleus/store/rdbms/sql/SQLStatement.java | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/java/org/datanucleus/store/rdbms/query/BulkFetchHelper.java b/src/java/org/datanucleus/store/rdbms/query/BulkFetchHelper.java
index 243e4ff..2246422 100644
--- a/src/java/org/datanucleus/store/rdbms/query/BulkFetchHelper.java
+++ b/src/java/org/datanucleus/store/rdbms/query/BulkFetchHelper.java
@@ -144,6 +144,7 @@ public class BulkFetchHelper
             sqlMapper.compile();
 
             // Add EXISTS clause on iterator statement so we can restrict to just the owners in this query
+ existsStmt.setOrdering(null, null);
             BooleanExpression existsExpr = new BooleanSubqueryExpression(sqlStmt, "EXISTS", existsStmt);
             sqlStmt.whereAnd(existsExpr, true);
 
@@ -165,7 +166,7 @@ public class BulkFetchHelper
             // SELECT ELEM_TBL.COL1, ELEM_TBL.COL2, ... FROM ELEM_TBL
             // WHERE EXISTS (SELECT OWNER_TBL.ID FROM OWNER_TBL WHERE (queryWhereClause) AND ELEM_TBL.OWNER_ID = OWNER_TBL.ID)
             SQLStatement sqlStmt = iterStmt.getSQLStatement();
-
+
             // Generate the EXISTS subquery (based on the JDOQL/JPQL query)
             SQLStatement existsStmt = RDBMSQueryUtils.getStatementForCandidates(storeMgr, sqlStmt, candidateCmd,
                 datastoreCompilation.getResultDefinitionForClass(), ec, query.getCandidateClass(), query.isSubclasses(), query.getResult(), null, null);
@@ -176,6 +177,8 @@ public class BulkFetchHelper
             sqlMapper.compile();
 
             // Add EXISTS clause on iterator statement so we can restrict to just the owners in this query
+ // no ORDER BY for exists subclause (forbitten in Oracle)
+ existsStmt.setOrdering(null, null);
             BooleanExpression existsExpr = new BooleanSubqueryExpression(sqlStmt, "EXISTS", existsStmt);
             sqlStmt.whereAnd(existsExpr, true);
 
diff --git a/src/java/org/datanucleus/store/rdbms/sql/SQLStatement.java b/src/java/org/datanucleus/store/rdbms/sql/SQLStatement.java
index 542a005..94c7ac5 100644
--- a/src/java/org/datanucleus/store/rdbms/sql/SQLStatement.java
+++ b/src/java/org/datanucleus/store/rdbms/sql/SQLStatement.java
@@ -1483,7 +1483,7 @@ public class SQLStatement
      */
     public void setOrdering(SQLExpression[] exprs, boolean[] descending, String[] nullOrders)
     {
- if (exprs.length != descending.length)
+ if (exprs != null && descending != null && exprs.length != descending.length)
         {
             throw new NucleusException(LOCALISER.msg("052503", "" + exprs.length, "" + descending.length)).setFatal();
         }

Andy Jefferson added a comment - 21/Feb/14 03:24 PM
GitHub master and 3.2 have this diff now. thx

Andy Jefferson made changes - 21/Feb/14 03:24 PM
Field Original Value New Value
Status Open [ 1 ] Resolved [ 5 ]
Fix Version/s 3.2.12 [ 12101 ]
Fix Version/s 4.0.0.m1 [ 12094 ]
Resolution Fixed [ 1 ]
Andy Jefferson made changes - 01/Mar/14 07:39 PM
Status Resolved [ 5 ] Closed [ 6 ]