|
|
Implementation of LIMIT and OFFSET for Oracle |
Query page
Enter new bug
If you are using an Oracle DB of version 8.1.6 or above, you can use
the 'rank()' function to implement full support for LIMIT and OFFSET.
For example to query all rows between number 3 and 8 you can use:
select *
from (
select empno, ename, job, sal,
rank() over (order by sal) rnk
from emp
)
where rnk between 3 and 8
So this is how OQL queries should be transformed:
"SELECT o FROM Person o ORDER BY personId LIMIT $1 OFFSET $2"
->
select *
from (
select ... ,
rank() over (order by ...) rnk
from ...
)
where rnk between :2 and :2 + :1
"SELECT o FROM Person o ORDER BY personId LIMIT $1"
->
select *
from (
select ... ,
rank() over (order by ...) rnk
from ...
)
where rnk <= :1
It may be possible to optimize the simple case with 'LIMIT', but
without 'OFFSET' and without an 'order by' clause.
For this simple case you can use the 'rownum' in a statement like.
This is also compatible to older Oracle DBs like 8.1.5 or 7.3.4:
select *
from emp
where rownum <= :1
------- Additional Comment
#1 From
Werner Guttmann
2004-09-03 03:46 -------
Created an attachment (id=639)
Initial (partial) patch.
------- Additional Comment
#2 From
Werner Guttmann
2004-09-03 03:48 -------
Above patch is only a partial solution for the feature request described above.
As it is, it adds the outer select statement incl. the WHERE rnk ... clause.
With regards to the insertion of the
rank() over (order by ...) rnk
expression, I am a bit torn between inserting this into the SQL code already
generated at this time through StringBuffer.manipulation (e.g. before the FROM
identifier) or a more structured approach. Hmm ?
------- Additional Comment
#3 From
Werner Guttmann
2004-09-10 03:09 -------
Martin, looking at your decription above and at the Oracle manuals at
http://download-west.oracle.com/docs/cd/B14117_01/server.101/b10759/functions108.htm#sthref1577,
it looks like the rank() function requires an ORDER BY clause to be in place.
Question; what to do if one does not want to specify such a clause ?
------- Additional Comment
#4 From
Werner Guttmann
2004-09-10 03:11 -------
Pending a reply to my question above, here's an initial patch for review
------- Additional Comment
#5 From
Werner Guttmann
2004-09-10 03:12 -------
Created an attachment (id=647)
Initial patch.
------- Additional Comment
#6 From
Martin Fuchs
2004-09-10 11:27 -------
Yes, the rank functions needs a sort order to be able to generate the ranking
of datasets. So you will always have to add an order by clause.
There might be two ways to handle the case, if the Castor user doesn't
explicitly specify one:
a.) The first one, which I would prefer, is to issue an exception. This is
justifiable because it doesn't make much sense to try windowing a query result
set to some range [from .. to] without an defined order. The results would not
be predictable.
b.) Alternatively one might force a predictable (but somehow arbitrarily) sort
order by automatically generating a ORDER BY clause containing all SELECT
columns. To use the example from above, the query would become
select *
from (
select empno, ename, job, sal,
rank() over (ORDER BY EMPNO, ENAME, JOB, SAL) rnk
from emp
)
where rnk between 3 and 8
If we decided to use solution a.) {throwing expceptions}, I could also live
with creating a ORDER BY in the special case, if there exists a LIMIT clause
but no OFFSET clause in the OQL. This would be some kind of middle way between
a.) and b.) .
What do you think on this ideas?
By the way:
How do other database systems than Oracle handle the case of LIMIT/OFFSET
queries without any ORDER criteria?
------- Additional Comment
#7 From
Martin Fuchs
2004-09-10 12:42 -------
Hi Werner,
experimenting a bit with your patch I noticed my alternative b.) is not quite
correct. One should use only generate an ORDER BY using the identity columns of
the queried table, not all columns.
Btw. there is currently missing a coma in the SQL syntax.
It should be:
sql.append (", rank() over ( ");
instead of
sql.append (" rank() over ( ");
------- Additional Comment
#8 From
Werner Guttmann
2004-09-10 13:06 -------
Martin, thanks for pointing out the incorrect syntax wrt adding a rank()
function to the generated SQL statement. I have already changed the relevant
code section, and will provide a new patch once I have considered your
statements re: the use of an ORDER BY clause.
------- Additional Comment
#9 From
Werner Guttmann
2004-09-11 13:28 -------
Martin, I thin I am actually in favour of option a), i.e. throwing an exception.
Just looking at OracleQueryExpression.getStatement(), it looks like the method
signature has to be changed to include a throws clause, or more specifically
public String getStatement (boolean lock) throws XXXException {
I am not 100% sure yet whether to throw a (more general) OQlQueryException or a
SyntaxNotSupportedException, though this would be a bit misleading.
In addition, one would need to change the method signature in
JDBCQueryExpression as well.
------- Additional Comment
#10 From
Martin Fuchs
2004-09-12 14:35 -------
Well, I think a SyntaxNotSupportedException would be the right exception to
throw. The description "Exception thrown to indicate that a feature is not
supported by a particular database." matches the problem we are faced here. To
make it clear you can use a message like "LIMIT without ORDER BY" as argument
for the exception constructor.
------- Additional Comment
#11 From
Werner Guttmann
2004-09-13 12:18 -------
Created an attachment (id=654)
New patch
------- Additional Comment
#12 From
Werner Guttmann
2004-09-13 12:19 -------
Above patch is a complete patch, changing OracleQueryExpression as diuscussed
above, and addíng throws SyntaxNotSupportedException to
JDBCQueryExpression.getStatement().
------- Additional Comment
#13 From
Martin Fuchs
2004-09-13 13:30 -------
OK, this works for me when using the LIMIT keyword.
But when trying to use the OFFSET keyword there is a problem with the generated
SQL, for example:
select * from (
SELECT "EMP"."EMPNO","EMP"."ENAME","EMP"."JOB","EMP"."MGR","EMP"."HIREDATE","EMP
"."SAL","EMP"."COMM","EMP"."DEPTNO" , rank() over ( ORDER BY "EMP"."EMPNO" )
rnk FROM "EMP" ) where rnk between ? and ? + ?
But there are only two bind variables, e.g.:
oql.bind(limit);
oql.bind(offset);
The generated SQL should better counted place holders:
... where rnk between :2 and :1 + :2
Just another minor fix. The exception message should be "To use a LIMIT ..."
instead of "To use a LIMT ...".
------- Additional Comment
#14 From
Werner Guttmann
2004-09-13 13:59 -------
Martin, can you please confirm that your OQL looks similar to
"SELECT o FROM Person o ORDER BY personId LIMIT $1 OFFSET $2"
------- Additional Comment
#15 From
Martin Fuchs
2004-09-13 14:26 -------
Yes, this is a snippet of my test class:
// Query DEPT datasets
OQLQuery oql = db.getOQLQuery(
"SELECT x\n"+
"FROM scottJdo.Emp x\n"+
"ORDER BY EMPNO LIMIT $1 OFFSET $2"
);
oql.bind(2);
oql.bind(1);
QueryResults results = oql.execute(Database.ReadOnly);
try {
while(results.hasMore()) {
Emp emp = (Emp) results.next();
System.out.println("EMP.EMPNO="+emp.EMPNO);
System.out.println("");
}
} finally {
results.close();
oql.close();
}
If you need a full test case, I can post it tomorrow.
------- Additional Comment
#16 From
Werner Guttmann
2004-09-14 01:05 -------
Created an attachment (id=656)
Revised patch as per Martin's advice.
------- Additional Comment
#17 From
Martin Fuchs
2004-09-14 02:31 -------
The hardcoded bind variable positions of course work only when using no other
bind variables than LIMIT and OFFSET. Why not fix it for all cases in
ParseTreeWalker:
Index: ParseTreeWalker.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/oql/ParseTreeWalker.java
,v
retrieving revision 1.5
diff -u -p -d -w -b -r1.5 ParseTreeWalker.java
--- a/ParseTreeWalker.java 8 Sep 2004 09:12:25 -0000 1.5
+++ b/ParseTreeWalker.java 14 Sep 2004 09:28:55 -0000
at at -1130,7 +1130,7 at at public class ParseTreeWalker implements
paramNumber = new Integer(sqlExpr.substring(pos + 1));
ParamInfo paramInfo = (ParamInfo) _paramInfo.get(paramNumber);
paramInfo.mapToSQLParam( SQLParamIndex++ );
- sb.append( sqlExpr.substring( startPos, pos+1 ) );
+ sb.append( ":" + sqlExpr.substring( startPos+1, pos+2 ) );
startPos = endPos < 0 ? sqlExpr.length() : endPos;
pos = sqlExpr.indexOf("?", startPos);
}
at at -1166,7 +1166,7 at at private void addOffsetClause(ParseTreeNo
paramNumber = new Integer(sqlExpr.substring(pos + 1));
ParamInfo paramInfo = (ParamInfo) _paramInfo.get(paramNumber);
paramInfo.mapToSQLParam( SQLParamIndex++ );
- sb.append( sqlExpr.substring( startPos, pos+1 ) );
+ sb.append( ":" + sqlExpr.substring( startPos+1, pos+2 ) );
startPos = endPos < 0 ? sqlExpr.length() : endPos;
pos = sqlExpr.indexOf("?", startPos);
}
------- Additional Comment
#18 From
Werner Guttmann
2004-09-14 03:00 -------
Just wondering whether the ':1 syntax' is common across al RDBMS ?
------- Additional Comment
#19 From
Martin Fuchs
2004-09-14 03:40 -------
This is a good question. I justed searched the web and found, it is a feature
of the Oracle JDBC driver:
http://radio.weblogs.com/0118231/stories/2003/03/05/clarifyingSomeBindVariableCo
nfusion.html
I'm very Oracle biased. ;-)
With Oracle version 10 you can even use named bind variables like ":EMPNO"
or ":SAL":
http://www.oracle.com/technology/sample_code/tech/java/codesnippet/jdbc/namedpar
am/UsingOracleNamedParameterAPIs.html
So what to do?
The current bug here is only to iomplement LIMIT/OFFSET for Oracle connections.
Because of this the ":1" syntax can be used in this place. But ParseTreeWalker
is a common class.
I think it would be the best to move the code, which generates the SQL bind
syntax into the database specific classes, in this case OracleQueryExpression.
Is as side effect this should also enhance the performance for Oracle queries a
bit. I will work on a better proposal...
------- Additional Comment
#20 From
Martin Fuchs
2004-09-14 04:07 -------
OK, here is the new approach introducing a new function getBindSql() in
QueryExpression to be able to use the Oracle specific bind variable syntax:
Index: src/main/org/exolab/castor/persist/spi/QueryExpression.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/persist/spi/QueryExpression.
java,v
retrieving revision 1.3
diff -u -p -d -r1.3 QueryExpression.java
--- a/src/main/org/exolab/castor/persist/spi/QueryExpression.java 8 Sep
2004 09:09:00 -0000 1.3
+++ b/src/main/org/exolab/castor/persist/spi/QueryExpression.java 14 Sep
2004 10:57:36 -0000
at at -354,6 +354,11 at at public interface QueryExpression
*/
public boolean isOffsetClauseSupported();
+ /**
+ * create SQL bind string from OQL bind string
+ */
+ public String getBindSql(String bind_oql);
+
}
Index: src/main/org/exolab/castor/jdo/drivers/JDBCQueryExpression.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/drivers/JDBCQueryExpress
ion.java,v
retrieving revision 1.4
diff -u -p -d -r1.4 JDBCQueryExpression.java
--- a/src/main/org/exolab/castor/jdo/drivers/JDBCQueryExpression.java 8 Sep
2004 09:22:13 -0000 1.4
+++ b/src/main/org/exolab/castor/jdo/drivers/JDBCQueryExpression.java 14 Sep
2004 10:57:34 -0000
at at -611,6 +617,13 at at public class JDBCQueryExpression
}
-}
+ /**
+ * create SQL bind string from OQL bind string
+ */
+ public String getBindSql(String bind_oql)
+ {
+ return bind_oql.substring(0, 1); /// at todo why not just
return "?" ?
+ }
+}
Index: src/main/org/exolab/castor/jdo/drivers/OracleQueryExpression.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/drivers/OracleQueryExpre
ssion.java,v
retrieving revision 1.2
diff -u -p -d -r1.2 OracleQueryExpression.java
--- a/src/main/org/exolab/castor/jdo/drivers/OracleQueryExpression.java 19 Jan
2004 20:01:46 -0000 1.2
+++ b/src/main/org/exolab/castor/jdo/drivers/OracleQueryExpression.java 14 Sep
2004 10:57:34 -0000
at at -130,17 +160,86 at at public final class OracleQueryExpression
+ /**
+ * create SQL bind string from OQL bind string
+ * using the Oracle specific ":1" syntax with positioned bind variables
+ */
+ public String getBindSql(String bind_oql)
+ {
+ return ":" + bind_oql.substring(1);
+ }
+}
Index: src/main/org/exolab/castor/jdo/oql/ParseTreeWalker.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/oql/ParseTreeWalker.java
,v
retrieving revision 1.5
diff -u -p -d -r1.5 ParseTreeWalker.java
--- a/src/main/org/exolab/castor/jdo/oql/ParseTreeWalker.java 8 Sep 2004
09:12:25 -0000 1.5
+++ b/src/main/org/exolab/castor/jdo/oql/ParseTreeWalker.java 14 Sep 2004
10:57:36 -0000
at at -1130,7 +1130,8 at at public class ParseTreeWalker implements
paramNumber = new Integer(sqlExpr.substring(pos + 1));
ParamInfo paramInfo = (ParamInfo) _paramInfo.get(paramNumber);
paramInfo.mapToSQLParam( SQLParamIndex++ );
- sb.append( sqlExpr.substring( startPos, pos+1 ) );
+ String bind_sql = _queryExpr.getBindSql(sqlExpr.substring(startPos,
pos+2));
+ sb.append(bind_sql);
startPos = endPos < 0 ? sqlExpr.length() : endPos;
pos = sqlExpr.indexOf("?", startPos);
}
at at -1166,7 +1167,8 at at private void addOffsetClause(ParseTreeNo
paramNumber = new Integer(sqlExpr.substring(pos + 1));
ParamInfo paramInfo = (ParamInfo) _paramInfo.get(paramNumber);
paramInfo.mapToSQLParam( SQLParamIndex++ );
- sb.append( sqlExpr.substring( startPos, pos+1 ) );
+ String bind_sql = _queryExpr.getBindSql(sqlExpr.substring(startPos,
pos+2));
+ sb.append(bind_sql);
startPos = endPos < 0 ? sqlExpr.length() : endPos;
pos = sqlExpr.indexOf("?", startPos);
}
------- Additional Comment
#21 From
Martin Fuchs
2004-09-14 04:23 -------
One more additional change for OracleQueryExpression:
// add LIMIT/OFFSET clause - part 2
if ( _limit != null ) {
sql.insert (0, " select * from ( ");
if ( _offset != null ) {
sql.append (" ) where rnk between " + _offset + " and "
+ _limit + " + " + _offset + " - 1 ");
} else {
sql.append (" ) where rnk <= " + _limit + " ");
}
}
There should be am additional "-1" in the "between" expression, but this has
been my fault at the very first beginning.
------- Additional Comment
#22 From
Werner Guttmann
2004-09-17 02:29 -------
Created an attachment (id=671)
Final patch for review
------- Additional Comment
#23 From
Werner Guttmann
2004-09-17 02:31 -------
Above patch is the total of all patches discussed above. Martin, can you please
review. I've been able to run
'test castor.mysql.TC62'
successfully, so I think I am ready to commit this change.
------- Additional Comment
#24 From
Martin Fuchs
2004-09-17 05:23 -------
Created an attachment (id=672)
revised patch
------- Additional Comment
#25 From
Martin Fuchs
2004-09-17 05:24 -------
Created an attachment (id=673)
revised patch without superfluous parts
------- Additional Comment
#26 From
Martin Fuchs
2004-09-17 05:33 -------
Hi Werner,
attached you find my revised version of the patch. (sorry for the first version
with changes in oracle.xml)
I resolved my TODO comment in the first version, and changed three things:
- getBindSql() gets now only the OQL parameter string without the leading "?".
So we can in fact just return "?" for in JDBCQueryExpression.getBindSql().
- reworked the changes in addLimitClause() and addOffsetClause
- Now I'm using getBindSql() also in addWhereClause().
I already run the CTF test cases for Oracle: No problems other than that of
TC47 and TC66.
------- Additional Comment
#27 From
Werner Guttmann
2004-09-17 07:37 -------
> I already run the CTF test cases for Oracle: No problems other than that
> of TC47 and TC66.
I've already committed a separate patch for TC66.. Please run the tests again,
and you should notice only one error.
------- Additional Comment
#28 From
Werner Guttmann
2004-09-17 07:37 -------
Do your patches build upon mine ?
------- Additional Comment
#29 From
Martin Fuchs
2004-09-17 09:45 -------
Sure, I took your last patch, applied it to the CVS code, and changed it as
described.
Mhh, TC66 doesn't pass yet. Instead there is another problem with TC20:
junit.framework.AssertionFailedError: (is) bool value was not set
at jdo.TypeHandling.testBooleanIsMethod(TypeHandling.java:305)
at jdo.TypeHandling.runTest(TypeHandling.java:421)
at harness.CastorTestCase.run(CastorTestCase.java:154)
at harness.TestHarness.run(TestHarness.java:140)
at harness.TestHarness.run(TestHarness.java:140)
at Main.run(Main.java:152)
at Main.main(Main.java:226)
Is see this even with the unpatched sources.
------- Additional Comment
#30 From
Werner Guttmann
2004-09-17 10:55 -------
Martin, can I read this as TC62 passes successfully. And the problem with TC20
is known, and can be 'investigated' by reading bug 1689.
As it happened, I checked in a change to a CTF test case assuming that it
executed successfully after a new testXXX() method has been added. Turned out
that this test case had a runTest() method where the new testXXX() method had
not been added .. :-(.
------- Additional Comment
#31 From
Martin Fuchs
2004-09-17 16:59 -------
No, TC62 doesn't pass. It is not part of the standard 'castor.oracle' test
suite, so I didn't run it till now. So I inserted it into the Oracle test case
list, and had to see some problems:
1. The TestLimitClause test class needs to be extended by "order by id"
expressions to be able to be used for Oracle.
2. It tries to execute: assertEquals (LIMIT, results.size());
This doesn't work for Oracle. (see TC66)
3. There are still problems with the :1 / :2 bind variables, so using LIMIT
with OFFSET doesn't work. However the testLimit() passes when replacing the
results.size() assertion by a assertTrue(!results.hasMore()) after the fetch
loop.
4. I don't think the testLimitWithOffset() test is correct:
OQLQuery query = _db.getOQLQuery("select t from jdo.TestObject t order by
id limit $1 offset $2");
query.bind(LIMIT);
query.bind(OFFSET);
QueryResults results = query.execute();
assertNotNull (results);
// assertEquals (5, results.size());
for (int i = 1 + OFFSET; i <= 5; i++) {
testObject = (TestObject) results.next();
assertEquals(i, testObject.getId());
}
assertTrue(!results.hasMore());
Shouldn't the loop better be:
for (int i = 1 + OFFSET; i <= OFFSET+LIMIT; i++) {
------- Additional Comment
#32 From
Martin Fuchs
2004-09-18 03:53 -------
Created an attachment (id=677)
Fix for TC62
------- Additional Comment
#33 From
Martin Fuchs
2004-09-18 04:01 -------
OK, let's first fix TC62.
This is what I changed:
- Change the test OFFSET to 2 instead of the previous value 5. The previous
value was not very good, because you can't see probblems with the parameter
order of LIMIT and OFFSET, if LIMIT and OFFSET are equal.
- Don't use size() calls to try counting result objects, because this isn't
portable, and can't be used with Oracle DB connections. Instead just test
with 'assertTrue(!results.hasMore());' after fetching all rows in the loop.
- I removed all hardcoded '5' values and replaced them with the LIMIT or OFFSET
constants.
- Fixed the wrong loop in testLimitWithOffset() as described above.
- Inserted a comment explaining the problems with the current DB-dependent
implementation of LIMIT/OFFSET bind variable order.
After applying the fix for TestLimitClause.java above, you can activate TC62
for the Oracle suite:
Index: tests.xml
===================================================================
RCS file: /cvs/castor/castor/src/tests/tests.xml,v
retrieving revision 1.7
diff -u -p -d -r1.7 tests.xml
--- a/tests.xml 12 Sep 2004 20:06:55 -0000 1.7
+++ b/tests.xml 18 Sep 2004 10:54:13 -0000
at at -104,6 +104,7 at at
<case class="jdo.TimeStampMismatched" />
<case class="jdo.Size" />
<case class="jdo.Absolute" />
+ <case class="jdo.TestLimitClause" id="TC62" />
</category>
------- Additional Comment
#34 From
Martin Fuchs
2004-09-18 10:01 -------
Created an attachment (id=678)
last version of the patch?
------- Additional Comment
#35 From
Martin Fuchs
2004-09-18 10:11 -------
Here is my last version of this patch. It passes now the the Oracle and MySQL
test cases including TC62. Of course the following test cases fail: 02(MySQL),
47(Oracle), 20(Oracle+MySQL), 66(Oracle). But that are other problems.
I removed the Oracle specific :1/:2 bind parameters again, because they did not
work as expected. Instead I'm know using an expression like "... where rnk -
OFFSET between 1 and LIMIT".
Known little problems are:
- Using LIMIT and OFFSET together still requires an database specific order of
bind variables. For Oracle and MySQL you have to "limit $2 offset $1", and for
PostgreSQL you have to use "limit $1 offset $2", if afer that calling:
query.bind(LIMIT);
query.bind(OFFSET);
- The problem described at the bottom of
http://brainopolis.dnsalias.com/castorwiki/Wiki.jsp?page=OQLLimit still exists
(for all databases): You should use LIMIT only for simple queries, which don't
generate joined SQL queries.
------- Additional Comment
#36 From
Werner Guttmann
2004-09-23 13:57 -------
> Here is my last version of this patch. It passes now the the Oracle and
> MySQL test cases including TC62. Of course the following test cases
> fail: 02(MySQL), 47(Oracle), 20(Oracle+MySQL), 66(Oracle).
Using all the latest and greatest source code from CVS plus the total of all the
patches discussed here, for mySQL it's only TC02 failing (which afair is a known
issue). Let me post a complete patch, and I'd appreciate if you could run it
against Oracle.
------- Additional Comment
#37 From
Werner Guttmann
2004-09-23 14:27 -------
> - The problem described at the bottom of
> http://brainopolis.dnsalias.com/castorwiki/Wiki.jsp?page=OQLLimit still
> exists (for all databases): You should use LIMIT only for simple
>queries, which don't generate joined SQL queries.
How about throwing an exception if a LIMIT clause is used with more complex
queries, e.g. including a join. This should be achievable.
------- Additional Comment
#38 From
Werner Guttmann
2004-09-23 14:28 -------
> - Using LIMIT and OFFSET together still requires an database specific
> order of bind variables.
I honestly cannot think of any solution here.
------- Additional Comment
#39 From
Werner Guttmann
2004-09-23 14:36 -------
Created an attachment (id=688)
Complete patch (for review)
------- Additional Comment
#40 From
Werner Guttmann
2004-09-23 14:36 -------
Complete patch as discussed above.
------- Additional Comment
#41 From
Martin Fuchs
2004-09-24 07:09 -------
The Castor patch itself is OK, but you in changed the bind variable order
from "limit $2 offset $1" into "limit $1 offset $2" at the following line in
TestLimitClause.java:
+ OQLQuery query = _db.getOQLQuery("select t from jdo.TestObject t order
by id limit $1 offset $2");
+ //OQLQuery query = _db.getOQLQuery("select t from jdo.TestObject t
order by id limit $1 offset $2");
You should revert this to enable successfull passing of TC62 for MySQL and
Oracle.
------- Additional Comment
#42 From
Werner Guttmann
2004-09-24 12:35 -------
I didn't mean to check this in, to be honest, but at 1:30 a.m. things
unfortunately happen .. :-(. Will revert this code.
------- Additional Comment
#43 From
Werner Guttmann
2004-09-24 12:36 -------
Well, forget my last comment ... I desperately need some sleep.
------- Additional Comment
#44 From
Martin Fuchs
2004-09-26 05:06 -------
>> - The problem described at the bottom of
>> http://brainopolis.dnsalias.com/castorwiki/Wiki.jsp?page=OQLLimit still
>> exists (for all databases): You should use LIMIT only for simple
>>queries, which don't generate joined SQL queries.
>How about throwing an exception if a LIMIT clause is used with more complex
>queries, e.g. including a join. This should be achievable.
Yes, would be a possibility. This way at least no one gets wrong result data.
>> - Using LIMIT and OFFSET together still requires an database specific
>> order of bind variables.
>I honestly cannot think of any solution here.
Well, the order of bind values in OQLQueryImpl._bindValues or
OQLQueryImpl._paramInfo would have to be changed depending on the database type.
------- Additional Comment
#45 From
Martin Fuchs
2004-09-27 04:03 -------
Created an attachment (id=695)
DB type independend patch
------- Additional Comment
#46 From
Martin Fuchs
2004-09-27 04:06 -------
The new patch above now handles the order of LIMIT/OFFSET bind variable in a
database type independend manner by introducing a new function
postProcessParamInfo() in the interface QueryExpression. This may not be the
cleanest solution, but I think it solves the problem. I tested against Oracle
and MySQL.
------- Additional Comment
#47 From
Werner Guttmann
2004-09-27 13:46 -------
Committed as discussed above.
------- Additional Comment
#48 From
Werner Guttmann
2004-09-27 13:48 -------
I think we should still throw an exception when the LIMIT clause is used with
'complex' SQL statements in OracleQueryExpression. We could e.g. search the
generated SQL String for occurrences of JOIN, etc. Very simplistic, but hey,
this whole class is one single hack .. ;-).
------- Additional Comment
#49 From
Martin Fuchs
2004-09-27 14:19 -------
Thanks for the commit. The discussion about this patch got a bit long. Now I
can update the 1733-patch.
I think it would be enough to query '_joins.isEmpty()' to decide, if the query
if a simply one.
Btw: Looking for "JOIN" in the SQL you would not find much - Oracle would
support it since version 9, but Castor uses only the old syntax. ;-)
------- Additional Comment
#50 From
Werner Guttmann
2004-09-27 14:30 -------
Sure, _joins.isEmpty() makes for a better candidate than looking for a substring
'JOIN' .. ;-). But what I had in mind was OUTER or/and LEFT JOINS, as they do
occur literally.
Let's try to get bug 1733 out of our way, so that I can focus (again) on support
for lazy-loading 1:1 relations (bug 1676), deal with a bit of a problem with
read-only support for 1:1 relations (bug 1660) and eventually get my head around
support for FK as part of a PK.
------- Additional Comment
#51 From
Werner Guttmann
2004-09-27 14:32 -------
And do not forget to close this bug once you've verified its correctness .. ;-).
I think I shall ask a cent for every time I mention this ... ;-). For those of
us listening in without an appreciation of cents, that makes (almost) a pence.
------- Additional Comment
#52 From
Martin Fuchs
2004-09-27 15:16 -------
I know, this does not really belong here...
But I just saw there is missing a isLimitClauseSupported() function in
SQLServerQueryExpression.java, as it claims to be able to process LIMIT queries:
Index: src/main/org/exolab/castor/jdo/drivers/SQLServerQueryExpression.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/drivers/SQLServerQueryEx
pression.java,v
retrieving revision 1.2
diff -u -p -d -r1.2 SQLServerQueryExpression.java
--- a/src/main/org/exolab/castor/jdo/drivers/SQLServerQueryExpression.java
19 Jan 2004 20:01:46 -0000 1.2
+++ b/src/main/org/exolab/castor/jdo/drivers/SQLServerQueryExpression.java
27 Sep 2004 22:09:36 -0000
at at -140,7 +140,9 at at public final class SQLServerQueryExpress
return sql.toString();
}
+
+ public boolean isLimitClauseSupported () {
+ return true;
+ }
}
-
-
I can't test it, may be some one else has a MS SQL server running and can look
into this?
Another thing...
Class HsqlQueryExpression also tries to generate SQL including a LIMIT
expression. But my HSQL test DB (version 1.7.2.4) doesn't recognise this. Has
this been tested by some one?
------- Additional Comment
#53 From
Martin Fuchs
2004-09-27 15:38 -------
> And do not forget to close this bug once you've verified its correctness .. ;-
).
OK, correctness tested - verified - certificate given. :)
------- Additional Comment
#54 From
Werner Guttmann
2004-09-28 01:07 -------
> But I just saw there is missing a isLimitClauseSupported() function
> in SQLServerQueryExpression.java, as it claims to be able to process
> LIMIT queries:
If that's the case, what syntax would MS SQL Server be using for a LIMIT clause ?
------- Additional Comment
#55 From
Werner Guttmann
2004-09-28 01:08 -------
> Class HsqlQueryExpression also tries to generate SQL including a LIMIT
> expression. But my HSQL test DB (version 1.7.2.4) doesn't recognise
> this. Has this been tested by some one?
As far as I know, the person submitting the original enhancement request was
using HSQL. Anyhow, what problems are you seeing ? Can you post the stack trace
(or at least the error message) ?
------- Additional Comment
#56 From
Martin Fuchs
2004-09-28 01:22 -------
For SQL Server there is used the syntax "TOP n":
if ( _limit != null )
if ( _limit.equals("") == false )
sql.append("TOP ").append(_limit).append(" ");
I just issued simple queries using the HSQL query tool, for example:
select *
from Customer
where 1=1
limit 6 2
And I get an "unexpected token: LIMIT in statement [limit]" error message.
The following prints "Unexpected token: 5 in statement [5]":
select *
from Customer
top 5
------- Additional Comment
#57 From
Martin Fuchs
2004-09-28 01:30 -------
Found, what's wrong with HSQL.
The syntax should be:
select limit 0 3 *
from Customer
and:
select top 5 *
from Customer
------- Additional Comment
#58 From
Werner Guttmann
2004-09-28 02:41 -------
Can you please supply me with unified patches ?
------- Additional Comment
#59 From
Martin Fuchs
2004-09-28 03:03 -------
*lol* I don't yet have patches.
May be we should move this discussion and re-open bug 1221 for the HSQL patch.
It will also require some reordering of bind parameters. It's even more
complicated than on Oracle, because you have to move the LIMIT and OFFSET
paramaters at the first position - even bevore any parameters used in the WHERE
expressions.
------- Additional Comment
#60 From
Werner Guttmann
2004-09-28 04:03 -------
Agreed. And let's move the HSQL and MSSQL parts of this to separate (new) bugs.
Let's not reopen bug 1221, please.
And how come you cannot supply me with patches yet ? It's more than 60mins since
you've posted this ... ;-).
------- Additional Comment
#61 From
Werner Guttmann
2004-10-05 13:38 -------
*** Bug 1199 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.
|
|