castor

Patch for numbered parameters in SQL pass-through queries

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.0
  • Fix Version/s: 1.0.1
  • Component/s: JDO queries
  • Labels:
    None
  • Number of attachments :
    1

Description

While upgrading from Castor 0.9.3 to 1.0 I discovered that pass-through SQL queries containing repeated numbered parameters suddenly ceased to work. Here is a very basic example of such a query:

SELECT * FROM foo WHERE foo.field1 = $1 AND foo.field2 = $1

Looking at the debugging info I was able to establish that the second parameter was interpreted so as it was given as $2.

A couple of hours later I found the problem in org.exolab.castor.jdo.engine.OQLQueryImpl.createCall(). The method parses the provided SQL query, replaces all the dollars with question marks and throws away all the parameter numbers.

The correct behaviour would be to simply replace $n with ?n, which is then correctly interpreted by org.exolab.castor.util.SqlBindParser responsible for setting the actual parameter values.

The attached patch fixes the problem and additionally corrects a minor glitch a couple of lines below

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Werner Guttmann added a comment -

Thanks, Pawel. We'll be having a look at your patch once we've got our project infrastructure back, incl. e.g. SVN repo, etc.

Show
Werner Guttmann added a comment - Thanks, Pawel. We'll be having a look at your patch once we've got our project infrastructure back, incl. e.g. SVN repo, etc.
Hide
Permalink
Werner Guttmann added a comment -

Pawel, can you please knock up e.g. a small JUnit test (relative to src/bugs) that highlights the problem that you've encountered, and helps us to verify (once the patch has been applied) that the problem does not persist anymore. Most likely, this new test will be back-integrated into the Castor JDO CTF test suite at a later time. Having said that, you might as well amend an existing CTF test and thus spare me some work .. , if you feel adventerous.

Show
Werner Guttmann added a comment - Pawel, can you please knock up e.g. a small JUnit test (relative to src/bugs) that highlights the problem that you've encountered, and helps us to verify (once the patch has been applied) that the problem does not persist anymore. Most likely, this new test will be back-integrated into the Castor JDO CTF test suite at a later time. Having said that, you might as well amend an existing CTF test and thus spare me some work .. , if you feel adventerous.
Hide
Permalink
Werner Guttmann added a comment -

Committed patch as is, amending OQLTest.java (ctf.jdo.tc3x) at the same time to extend coverage of the CTF suite.

Show
Werner Guttmann added a comment - Committed patch as is, amending OQLTest.java (ctf.jdo.tc3x) at the same time to extend coverage of the CTF suite.
Hide
Permalink
Pawel Pesz added a comment -

Sorry Werner for not having reacted to your comments but I forgot to configure e-mail notification for this issue. Thanks for a very swift response

Show
Pawel Pesz added a comment - Sorry Werner for not having reacted to your comments but I forgot to configure e-mail notification for this issue. Thanks for a very swift response
Hide
Permalink
Werner Guttmann added a comment -

You are welcome. What surprises me, though, is that you (as the original reporter) do not get notified about any comments being added to the issue ...

Show
Werner Guttmann added a comment - You are welcome. What surprises me, though, is that you (as the original reporter) do not get notified about any comments being added to the issue ...
Hide
Permalink
Werner Guttmann added a comment -

Pawel, I just reconfigured the notification scheme for Castor's Jira, and you should get a small notification about this (my) comment ... .

Show
Werner Guttmann added a comment - Pawel, I just reconfigured the notification scheme for Castor's Jira, and you should get a small notification about this (my) comment ... .
Hide
Permalink
Pawel Pesz added a comment -

Got it

Show
Pawel Pesz added a comment - Got it

People

  • Assignee:
    Werner Guttmann
    Reporter:
    Pawel Pesz
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: