History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: CASTOR-363
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Blocker Blocker
Assignee: Werner Guttmann
Reporter: ogura
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
castor

LIMIT clause causes error with PostgreSQL 7.3 and HSQLDB

Created: 20/Feb/03 06:45 PM   Updated: 18/Apr/05 07:26 PM
Component/s: JDO
Affects Version/s: 0.9.4.2
Fix Version/s: None

Time Tracking:
Not Specified

Environment:
Operating System: All
Platform: All

Bugzilla Id: 1221


 Description  « Hide
CastorJDO 0.9.4.3 generates 'LIMIT #,#' style SQL when OQL contains LIMIT clause.
But PostgreSQL 7.3 doesn't support this style, as mentioned below URL.

http://developer.postgresql.org/docs/postgres/release-7-3.html#AEN24452

Backend server returns error message:

ERROR: LIMIT #,# syntax not supported.
Use separate LIMIT and OFFSET clauses.

I created patch to solve this problem.

Index: src/main/org/exolab/castor/jdo/drivers/PostgreSQLQueryExpression.java
===================================================================
RCS file:
/cvs/castor/castor/src/main/org/exolab/castor/jdo/drivers/PostgreSQLQueryExpression.java,v
retrieving revision 1.13
diff -u -w -r1.13 PostgreSQLQueryExpression.java
— src/main/org/exolab/castor/jdo/drivers/PostgreSQLQueryExpression.java 19 Apr
2001 23:18:40 -0000 1.13
+++ src/main/org/exolab/castor/jdo/drivers/PostgreSQLQueryExpression.java 21 Feb
2003 02:39:20 -0000
at at -49,6 +49,7 at at

import java.util.Enumeration;
import java.util.Hashtable;
+import java.util.StringTokenizer;
import java.util.Vector;
import org.exolab.castor.jdo.engine.JDBCSyntax;
import org.exolab.castor.persist.spi.PersistenceFactory;
at at -77,8 +78,11 at at

sql = getStandardStatement( lock, false );

  • if ( _limit != null )
  • sql.append(JDBCSyntax.Limit).append(_limit);
    + if ( _limit != null ) { + StringTokenizer tokenizer = new StringTokenizer(_limit, ",", false); + sql.append(JDBCSyntax.Offset).append(tokenizer.nextToken()); + sql.append(JDBCSyntax.Limit).append(tokenizer.nextToken()); + }

// Use FOR UPDATE to lock selected tables.
if ( lock )
Index: src/main/org/exolab/castor/jdo/engine/JDBCSyntax.java
===================================================================
RCS file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/engine/JDBCSyntax.java,v
retrieving revision 1.7
diff -u -w -r1.7 JDBCSyntax.java
— src/main/org/exolab/castor/jdo/engine/JDBCSyntax.java 17 Dec 2000 15:40:04
-0000 1.7
+++ src/main/org/exolab/castor/jdo/engine/JDBCSyntax.java 21 Feb 2003 02:39:20 -0000
at at -89,5 +89,7 at at

public String Limit = " LIMIT "; //Alex

+ public String Offset = " OFFSET ";
+
}



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
lance - 28/Apr/03 02:31 PM
Can you attach an example of your OQL and how you are binding to it? I have
users from Postgresql to mySQL to HSQLDB, and all have somewhat different LIMIT
syntax. My research of Castor's code doesn't reveal support for these different
syntaxes.

Thanks.


ogura - 30/Apr/03 02:11 AM
Below code is a sample with PostgreSQL.

String query = "SELECT o FROM Person o ORDER BY personId LIMIT $1, $2";
OQLQuery oql = db.getOQLQuery(query);
oql.bind(0);
oql.bind(10);

This code generates SQL shown below:

SELECT ... FROM "person" ORDER BY "person"."personid" OFFSET ? LIMIT ? FOR UPDATE

Because I'm only using Castor with PostgreSQL,
I don't know MySQL or HSQLDB related function.
sorry.


lance - 30/Apr/03 10:40 AM
This bug prevents us (RollerWeblogger.org) from using LIMIT effectively (kills
HSQLDB dead) - which has significant performance implications.

For the problem I'm seeing I think you are on the right track; a solution came
to mind this morning that uses some of your solution.

Add JDBCSyntax " OFFSET " as you suggested. But also add it to the the
ParseTreeWalker (or wherever) such that a new variable _offset is set (much as
_limit is now). Then the various XQueryExpression classes could have their own
implementations. The Postgresql one would look like

if ( _limit != null )

{ sql.append(JDBCSyntax.Limit).append(_limit); if (_offset != null) sql.append(JDBCSyntax.Offset).append(_offset); }

Where the MySQL version would look like:

if ( _limit != null )

{ if (_offset != null) sql.append(_offset).append(","); sql.append(JDBCSyntax.Limit).append(_limit); }

HSQLDB doesn't support a single-argument LIMIT...

if ( _limit != null )
{
if (_offset != null)

{ sql.append(_offset).append(" "); // hsqldb doesn't use comma sql.append(JDBCSyntax.Limit).append(_limit); }

else

{ sql.append(" TOP ").append(_limit); }

}

All queries could then be written
"SELECT o FROM Person o ORDER BY personId LIMIT $1 OFFSET $2"


lance - 30/Apr/03 10:53 AM
Ugh, I messed up the last two examples:

Where the MySQL version would look like:
if ( _limit != null )

{ sql.append(JDBCSyntax.Limit); if (_offset != null) sql.append(_offset).append(","); sql.append(_limit); }

HSQLDB doesn't support a single-argument LIMIT...
if ( _limit != null )
{
if (_offset != null)

{ sql.append(JDBCSyntax.Limit); sql.append(_offset).append(" "); // hsqldb doesn't use comma sql.append(_limit); }

else

{ sql.append(" TOP ").append(_limit); }

}


lance - 04/May/03 10:14 AM
Created an attachment (id=277)
I think this patch will address the issues - unsure how to test all conditions.

Werner Guttmann - 07/Aug/04 11:23 AM
Just re-assigned this bug to myself.

Werner Guttmann - 07/Aug/04 11:25 AM
Lance et alias, I'll work to get this patch integrated with the latest and
greatest code in CVS (where some things might have changed in the meantime,
given that a year has passed since the bug had been raised). I'll make progress
visible through this bug report.

Thinking aloud, it would be really great if we could extend the offered
functionality to other RDBMS such as DB2 or Oracle.


Werner Guttmann - 08/Aug/04 12:59 PM
I have managed to successfully apply above patch to the current codebase.
Testing (on mySQL) looks fine to me, being able to restrict the size of the
result set according to the spec.

I do have one problem, though: it looks like I can only use queries such as

select n from test1221.Account n limit $1, $2

bug get an exception for a query

select n from test1221.Account n limit 5, 5

Exception:
org.exolab.castor.jdo.oql.OQLSyntaxException: An incorrect token type was found
near 5 (found LONG_LITERAL, but expected DOLLAR)
at org.exolab.castor.jdo.oql.Parser.match(Parser.java:208)
at org.exolab.castor.jdo.oql.Parser.queryParam(Parser.java:916)
at org.exolab.castor.jdo.oql.Parser.limitClause(Parser.java:1000)
at org.exolab.castor.jdo.oql.Parser.getParseTree(Parser.java:167)
at org.exolab.castor.jdo.engine.OQLQueryImpl.create(OQLQueryImpl.java:244)
at org.exolab.castor.jdo.engine.DatabaseImpl.getOQLQuery(DatabaseImpl.java:422)
at test1221.Test1221.testLoadAccountOQL(Test1221.java:65)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:324)
at junit.framework.TestCase.runTest(TestCase.java:154)
at junit.framework.TestCase.runBare(TestCase.java:127)
at junit.framework.TestResult$1.protect(TestResult.java:106)
at junit.framework.TestResult.runProtected(TestResult.java:124)
at junit.framework.TestResult.run(TestResult.java:109)
at junit.framework.TestCase.run(TestCase.java:118)
at junit.framework.TestSuite.runTest(TestSuite.java:208)
at junit.framework.TestSuite.run(TestSuite.java:203)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:421)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:305)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:186)

Any idea why ? I had a look at ParseTreeWalker, and there's CASE statements for
LONG_LITERAL, etc. But they never get hit ....


lance - 08/Aug/04 05:27 PM
If you look at one of my comments you'll see I ended it with:

All queries could then be written
"SELECT o FROM Person o ORDER BY personId LIMIT $1 OFFSET $2"

So if you have replicated my solution (on the new codebase) your query should
look like

select n from test1221.Account n limit $1 offset $2

Does capitilization of the keywords matter?


Werner Guttmann - 09/Aug/04 12:32 AM
Lance, I agree with you in that I expected to be able to use the OFFSET literal
in my queries (as a result of the additions to Token.java et alias), but as I
have mentioned above, the only way I could get it to work was with the
following syntax:

select n from test1221.Account n limit $1, $2

And no, capitalization didn't matter as I've tried this as well.


lance - 09/Aug/04 04:57 AM
I haven't looked a tthe code since I submitted my patch, and I doubt I'll have
time soon. The Roller project is no longer using Castor (for this and other
reasons) so if you want to back out your changes it is fine by me.

Which patch were you using, the original (inline) or mine at
http://bugzilla.exolab.org/showattachment.cgi?attach_id=277 ?


Werner Guttmann - 09/Aug/04 07:25 AM
Tha latter, as attached by yourself on 05/04/2003. Don't worry, I'll find a
solution one way or the other .. : Let#s see what Bruce might have to say as
he sees the TreeWalker as one of his personal babies.

Werner Guttmann - 10/Aug/04 02:46 PM
Created an attachment (id=619)
Modified patch

Werner Guttmann - 10/Aug/04 02:48 PM
I've just attached a (slightly .. ) modified patch that makes the LIMIT
clause working as discussed above. With this patch applied, one can now specify
OQLQueries such as

"SELECT o FROM Person o ORDER BY personId LIMIT $1 OFFSET $2"

and

"SELECT o FROM Person o ORDER BY personId LIMIT $1"


Bruce Snyder (bugzilla) - 11/Aug/04 06:42 AM
I was working with this last week and I thought I left a comment here about it.
IIRC, that must have been when my browser crashed on me ;-/.

Werner, this patch looks good. Just get rid of the System.out.println() calls.
How about a quick test case?


lance - 11/Aug/04 06:04 PM
Thanks for working on this Werner. Even though I no longer use Castor its nice
to know this feature will be available. Pester Crazy Bob Lee for a LIMIT
implementation for Oracle (he blogged about it a long time ago, back in 2003).

Werner Guttmann - 12/Aug/04 12:08 AM
JUnit test case does exist, and I'll get rid of the System.out.println().
Though I'd love to know where they came from originally .. .

Werner Guttmann - 12/Aug/04 05:07 AM
But I thought I had replaced them anyhow when starting to look at the patch.
Well, eagle-eyed Bruce always betas me here ... .

Werner Guttmann - 13/Aug/04 01:06 AM
Created an attachment (id=620)
Proposed test case

Werner Guttmann - 13/Aug/04 01:07 AM
Here's the TestCase I have been using. I guess we should be adding this to the
test suite, correct ?

Werner Guttmann - 13/Aug/04 03:39 AM
Committed patch as above. I'd appreciate if somebody could briefly review the
added functionality abd close this bug.

Werner Guttmann - 17/Aug/04 12:51 PM
Created an attachment (id=622)
Patch for updating the documentation on OQL

Werner Guttmann - 17/Aug/04 12:52 PM
Above patch updates the documentation for OQL by adding a limit and an offset
clause to the grammar spec.

At the sime time, I added a 'News' section at the top, and an 'Examples' section
at the bottom.


Werner Guttmann - 08/Sep/04 03:35 AM
Created an attachment (id=646)
Proposed CTF test

Werner Guttmann - 23/Sep/04 01:25 PM
Closed.

Werner Guttmann - 23/Sep/04 01:25 PM
      • Bug 1038 has been marked as a duplicate of this bug. ***

Martin Fuchs - 28/Sep/04 03:19 AM
The syntax currently used for HSQL queries with LIMIT/OFFSET isn't correct.

For queries with LIMIT without OFFSET should be for example:

select top 5 * – 5=LIMIT
from Customer

The syntax for HSQL queries with LIMIT and OFFSET should be:

select limit 0 3 * – 0=OFFSET 3=LIMIT
from Customer

The implementation will require reordering of bind parameters. It's even more
complicated than on Oracle/MySQL, because you have to move the LIMIT and OFFSET
paramaters at the first position - even bevore any parameters used in the WHERE
expressions.


Martin Fuchs - 28/Sep/04 08:39 AM
Created an attachment (id=701)
fix for HSQL LIMIT implementation including TC62 for HSQL

Werner Guttmann - 28/Sep/04 02:17 PM
Martin, can you please have a close look at the patch provided by you. There's a
couple of problems with it:

a) Some files appear to be patched twice ?
b) For some files, the unified notation seems to be broken.

Any idea what's wrong ?


Martin Fuchs - 30/Sep/04 03:27 AM
You are right. Somehow I put the HsqlQueryExpression changes twice in the
patch. However they seem to be identically.

Please first commit the cleanup patch of entry 1753. I already tested it
against Oracle, MySQL and HSQL.
After that I can merge my sources, and update this patch here.


Werner Guttmann - 30/Sep/04 12:59 PM
Will commit bug 1753 tonight, if that's alright for you. And a new patch would
really make sense ... as I really, really do not have the time to play around
with hard-to-apply patches at the moment. Too many bugs to be worked on at the
moment.

Martin Fuchs - 01/Oct/04 04:47 AM
Created an attachment (id=709)
activated TC62 for HSQL, reorderParamInfo() removed, some comments fixed

Werner Guttmann - 01/Oct/04 12:50 PM
Patch applied as provided. Please close this bug report once you've assessed its
validity.

Martin Fuchs - 07/Oct/04 07:23 AM
The commit is OK. However we see now the key generator exceptions also here,
when running the HSQL test case:

D:\java\castor-1221>run-tests -verbose .hsql
DEBUG - Trying to load configuration file from file:/D:/java/castor-
1221/build/tests/castor.properties

testsuite: castor
testsuite: hsql
Test: TC62 Test limit clause .FATAL - A fatal exception occurred:
org.exolab.castor.mapping.MappingException: The key generator
org.exolab.castor.jdo.drivers.SequenceKeyGenerator is not compatible with the
persistence factory hsql.
FATAL - A fatal exception occurred: org.exolab.castor.mapping.MappingException:
Key generator org.exolab.castor.jdo.drivers.IdentityKeyGenerator doesnt support
SQL type {1}.
FATAL - A fatal exception occurred: org.exolab.castor.mapping.MappingException:
The key generator org.exolab.castor.jdo.drivers.SequenceKeyGenerator is not
compatible with the persistence factory hsql.
FATAL - A fatal exception occurred: org.exolab.castor.mapping.MappingException:
The key generator org.exolab.castor.jdo.drivers.SequenceKeyGenerator is not
compatible with the persistence factory hsql.
FATAL - A fatal exception occurred: org.exolab.castor.mapping.MappingException:
The key generator org.exolab.castor.jdo.drivers.SequenceKeyGenerator is not
compatible with the persistence factory hsql.

Time: 3,453

OK (1 test)

Couldn't it be this is because of the recent log4j configuration changes?
The test itself runs without problems. Just the FATAL messages are new.