Bugzilla Version 2.16.7
Bugzilla Bug 1711
  master detail OQL queries - explicit sort order for SQL queries with joined tables
     Query page      Enter new bug
Bug#: 1711   Platform:   Reporter: martin-fuchs at gmx dot net (Martin Fuchs)
Product:   OS:   Add CC:
Component:   Version:   CC:
Remove selected CCs
Status: NEW   Priority:  
Resolution:   Severity:  
Assigned To: ferret at frii dot com (Bruce Snyder)   Target Milestone:  
URL:
Summary:

Attachment Type Modified Status Actions
Patch for src/main/org/exolab/castor/jdo/drivers/OracleQueryExpression.java patch 2004-08-06 14:00 none Edit
OrderTest.java text/plain 2004-09-03 04:31 none Edit
A.java text/plain 2004-09-03 04:31 none Edit
B.java text/plain 2004-09-03 04:31 none Edit
jdo-conf.xml text/plain 2004-09-03 04:32 none Edit
mapping.xml text/plain 2004-09-03 04:32 none Edit
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 1711 depends on: Show dependency tree
Show dependency graph
Bug 1711 blocks:
Votes: 0    Show votes for this bug    Vote for this bug

Additional Comments:


Leave as NEW 
Accept bug (change status to ASSIGNED)
Resolve bug, changing resolution to
Resolve bug, mark it as duplicate of bug #
Reassign bug to
Reassign bug to owner of selected component

View Bug Activity   |   Format For Printing

Description: Opened: 2004-08-06 13:59

While testing an application we had to see, Castor has sometimes problems to 
read objects with master-detail connections. It doesn't find all children 
records in arrays, vectors or any other collections.
I traced down this problem to the functions nextIdentity() and fetchRaw() in 
class SQLEngine. This functions expect datasets with equal identity keys 
(e.g. "PKA") to be fetched in continuous order.

But this guaranteed for SQL statements like:

select A.PKA, B.PKB
from A, B
where A.PKA = B.PKA

This is at least the behaviour of Oracle databases.
I am not sure about other systems.
So I implemented the fix only for the class OracleQueryExpression.
May be other DBs also they need a similar fix.

The attached patch adds an explicit explicit sort order using the identity 
columns when joining tables to make dataset order predictable.
If there is specified an other ORDER BY expression, this is not changed.

------- Additional Comment #1 From Martin Fuchs 2004-08-06 14:00 -------
Created an attachment (id=615)
Patch for src/main/org/exolab/castor/jdo/drivers/OracleQueryExpression.java

------- Additional Comment #2 From Bruce Snyder 2004-09-02 20:05 -------
Martin, do you have any kind of a test case to prove this problem? If so, please
post it. 

------- Additional Comment #3 From Martin Fuchs 2004-09-03 00:02 -------
The problem is, you can't reproduce the missing order reliably.
However you can try if you see the behaviour using this SQL script:

drop table B
/
drop table A
/

create table A(NR_A number)
/
ALTER TABLE A ADD CONSTRAINT PKA PRIMARY KEY(NR_A)
/

create table B(NR_A number, NR_B number)
/
ALTER TABLE SCOTT.B ADD CONSTRAINT FK_A_B
    FOREIGN KEY(NR_A) REFERENCES A(NR_A)
/

insert into A(NR_A) values(1)
/
insert into A(NR_A) values(2)
/
insert into A(NR_A) values(3)
/
insert into A(NR_A) values(4)
/
insert into A(NR_A) values(5)
/
insert into A(NR_A) values(6)
/
insert into A(NR_A) values(7)
/
insert into A(NR_A) values(8)
/
commit
/

insert into B(NR_A, NR_B) values(2, 1)
/
insert into B(NR_A, NR_B) values(2, 2)
/

insert into B(NR_A, NR_B) values(1, 1)
/
insert into B(NR_A, NR_B) values(2, 3)
/
insert into B(NR_A, NR_B) values(3, 2)
/
insert into B(NR_A, NR_B) values(3, 3)
/
insert into B(NR_A, NR_B) values(3, 1)
/

insert into B(NR_A, NR_B) values(1, 2)
/
insert into B(NR_A, NR_B) values(1, 3)
/

commit
/



select A.NR_A, B.NR_B
from A, B
where A.NR_A = B.NR_A
-- no ORDER BY
/


What I get as result of the SELECT is for example:

NR_A    NR_B
2	1
2	2
1	1   <- (*)
2	3
3	2
3	3
3	1
1	2
1	3

Note the marked(*) data set 1/1. It is fetched between 2/2 and 2/3.
But Castor would expect it in continuous order with 1/2 and 1/3.

When inserting an ORDER BY clause a Castor-compatible result set is fetched:

select A.NR_A, B.NR_B
from A, B
where A.NR_A = B.NR_A
order by A.NR_A
/

NR_A    NR_B
1	1
1	2
1	3
2	1
2	3
2	2
3	2
3	3
3	1

------- Additional Comment #4 From Martin Fuchs 2004-09-03 04:05 -------
If you compute statistics for table A the behaviour can be reproduced more 
reliable:

analyze table A compute statistics
/

------- Additional Comment #5 From Martin Fuchs 2004-09-03 04:31 -------
Created an attachment (id=640)
OrderTest.java

------- Additional Comment #6 From Martin Fuchs 2004-09-03 04:31 -------
Created an attachment (id=641)
A.java

------- Additional Comment #7 From Martin Fuchs 2004-09-03 04:31 -------
Created an attachment (id=642)
B.java

------- Additional Comment #8 From Martin Fuchs 2004-09-03 04:32 -------
Created an attachment (id=643)
jdo-conf.xml

------- Additional Comment #9 From Martin Fuchs 2004-09-03 04:32 -------
Created an attachment (id=644)
mapping.xml

------- Additional Comment #10 From Martin Fuchs 2004-09-03 04:36 -------
The attached test program queries the tables A and B both using Castor and 
plain JDBC.

With the unpatched Castor version (that of 05.08.2004 in CVS), I can see the 
problem. It doesn't report correct object counts for the OQL selects:

A-Sum [Castor]: 10 (should be the same as the following number)
A-Sum [SELECT]: 8
B-Sum [Castor]: 9 (should be the same as the following number)
B-Sum [SELECT]: 15



With my patch the last output lines are correct:

A-Sum [Castor]: 8 (should be the same as the following number)
A-Sum [SELECT]: 8
B-Sum [Castor]: 9 (should be the same as the following number)
B-Sum [SELECT]: 9

------- Additional Comment #11 From Werner Guttmann 2004-09-03 04:42 -------
Excuse my ignorance, but what if somebody specified an ORDER BY clause that
would prevent the (patched) code from performing successfully, iow an ORDER BY
clause that isn't 'Castor-compatible' ?

------- Additional Comment #12 From Bruce Snyder 2004-09-03 04:52 -------
Thanks for the test. This is exaclty what I needed. I was never quite able to
figure out why results.size() was incorrect. I always knew that it was giving a
total number of objects found in the graph but I didn't go far enough to see
that the joins were affecting it in this way. 

My only question is exactly what Werner just asked. I tried to commit this a
moment ago and Bugzilla reported a Mid-Air Collision because the bug had been
updated while I was typing these comments. 

------- Additional Comment #13 From Martin Fuchs 2004-09-03 05:41 -------
> Excuse my ignorance, but what if somebody specified an ORDER BY clause that
> would prevent the (patched) code from performing successfully, iow an ORDER BY
> clause that isn't 'Castor-compatible' ?

Yes, this is still a problem. If the user specifies an order clause, it has to 
include the identity columns of the master table as first sort criterium. One 
solution would be to report an error, if this is not the case.
Of course the best solution would be to fix the algorithm in SQLEngine. But I 
fear, this isn't an easy task. For example it would imply to resize arrays on 
the fly when detecting more objects for an identity processed previously.
Ehm - I hope, I am clear enough, and you can understand, what I want to says 
with this.  ;-)

------- Additional Comment #14 From Martin Fuchs 2004-09-10 11:55 -------
Another problem, which is not handled quite satisfactory by my proposed patch 
(It's more a work around than a fire and forget solution), is the handling of 
database locks:

-            sql.append( " FOR UPDATE" );
+            if (!use_order_by)	// Locking by "FOR UPDATE" cannot be used with 
ORDER BY at the same time. 
+                sql.append( " FOR UPDATE" );
+

I chose to ignore the lock request.
Another way would be to throw an exception, because locks using "FOR UPDATE" in 
Oracle queries are not compatible to the use of an ORDER BY clause. But this 
would disallow any locks for OQL queries with master/detail data.


Perhaps we should better fix SQLEngine algorithm (at least as a long term 
solution). I might give it a try if I find some time to look deeper into this.
However the fix already posted is better then nothing and might save a few 
people data damage.

------- Additional Comment #15 From Martin Fuchs 2004-09-16 07:20 -------
There would be at least two different ways to come up with a solution without 
adding additional ORDER BY clauses to the generated SQL:

- "lazy loading" of detail data:
Don't use outer joins to fetch master and detail data at once.
Of course this would decrease the performance of SQL queries since we need to 
execute many more queries: one for the master table and as much for the detail 
table as there are identities in the master table.

- fetch all data of the unchanged outer join queries at once:
Currently the result objects are fetched once by once when calling 
QueryResults.hasMore() or QueryResults.next(). If all objects would internally 
be fetched at once, this would make possible to collect any detail data 
relating to the master data objects without missing incomming un-ordered SQL 
datasets. The resulting objects could be moved into the cache while reading the 
result set, however it has to be guarantied, all objects have space there. 
Sicne this solution could be a very memory consuming task, it should only be 
used for the case of 1:n queries. The algorithm for simple queries could remain 
unchanged.

Any better ideas?

------- Additional Comment #16 From Werner Guttmann 2004-09-23 15:00 -------
I do not know whether you've all seen Martin's comments above, and I am sure we
have already seen similar threads at this very place .. but I'd like to see
comments. And anyhow, does anybody know any bug numbers out there that we can
link to this one ?

------- Additional Comment #17 From Gregory Block 2004-09-27 05:11 -------
I have an instinctive feeling that this may well be related to another problem I've seen here, but can't 
recreate reliably, on MS SQL.  I was even seeing it in other cases - but in most of those cases, I've since 
manually split the load operation so that the objects are no longer dependent.

Gut instinct says this is actually a current issue on more than just oracle.

------- Additional Comment #18 From Bruce Snyder 2004-10-14 21:04 -------
As you state, Martin, one solution is to fetch all data using multiple queries
or using one great big query with some pretty awful joins. For a long time I've
been curious to investigate offering users a configurable option of which of
these strategies to use. Some people don't mind multiple queries while others
do. And different situations influence this issue differently. What about
offering this by way of a property in the castor.properties file? There is
really no easy answer to this issue. It's similar to the LIMIT/OFFSET isssue
with Oracle where we finally just agreed to support that feature *only* in
Oracle 8.1.6 +. Comments? 

------- Additional Comment #19 From Werner Guttmann 2004-10-16 02:29 -------
I think that's the right way to go about it, by making it configurable. Though I
do not agree that castor.properties is the right place to configure this. Imho,
a more granular approach is required. Why not overload (again?) the various
persistence-related methods of Database (as well) ? Iow, allow users to
configure a default mode in castor.properties, and give them means to override
this default on a per 'query' base. Hmm ?

------- Additional Comment #20 From Martin Fuchs 2004-10-16 03:08 -------
Yes, creating a new configuration switch is the way to make all users happy. 
But of course it will increase the complexity even more. You will have to 
implement and maintain both possibilities. I also think to introduce this 
switch on a per query base is more convenient. This enables to optimize the 
performance for the particular cases. If we add such a switch I would vote to 
be the method "lazy loading of detail data" the default. This is the most 
general approach. Querying all data of a complete table at once can use an 
unpredictable amount of memory and can take quite long, depending on the 
available data.

So what would be the allowed values of this new switch?
Let me enumerate all methodes describes in the comments above:

enum DETAIL_DATA_METHOD {
  DDM_LAZY_LOAD,    // fetch master data one by one in next() calls, with 
relaited detail data
  DDM_ALL_AT_ONCE,  // fetch all data in a single query at once, and process 
all at the execute() call
  DDM_FORCE_ORDER   // sort data using the identity of the master data to get 
all related detail data in sequence
}

The "all at once" and "force order" methods could be combined. However we 
should not create even more possibilities, and confain them to a meaningfull 
subset.

------- Additional Comment #21 From Werner Guttmann 2004-10-16 03:44 -------
Let me just have a look at the way passing access modes to e.g. Database.load()
is handled. As far as I remember, there's two options with this. Either specify
an access mode in the mapping file (access="read-only") or specify one at the
method level. Bruce, correct me if I am wrong.

------- Additional Comment #22 From Martin Fuchs 2004-10-30 03:33 -------
*** Bug 986 has been marked as a duplicate of this bug. ***

     Query page      Enter new bug
This is Bugzilla: the Mozilla bug system. For more information about what Bugzilla is and what it can do, see bugzilla.org.
Actions: New | Query | bug # | Reports   New Account | Log In