zhangjingl02 / activejdbc

Automatically exported from code.google.com/p/activejdbc
0 stars 0 forks source link

find(SQL, Object) does not handle ?s in raw SQL properly #193

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When the raw SQL methods are used, and the SQL looks like

select x.* from X where fieldName = 'testing?'

the lack of a parameter object passed to find() <DB.java, line 389> causes an 
exception to be thrown unnecessarily, despite the SQL being valid

What version of the product are you using? On what operating system?

version 1.4.2

?s surrounded by quotes should be ignored for the purpose of counting 
parameters. There does not seem to be a way to escape the ?s either.

Using find() for such a simple SQL statement isn't ideal, but the code also 
handle more complex statements, so this wasn't anticipated

Original issue reported on code.google.com by dr.fra...@gmail.com on 17 Apr 2013 at 3:43

GoogleCodeExporter commented 9 years ago
not sure I understand this, can you please provide a complete code snippet that 
can cause an issue? 

Original comment by i...@polevoy.org on 17 Apr 2013 at 4:19

GoogleCodeExporter commented 9 years ago
Sorry for the confusion. I did not debug it in action, but am sure the the call 
to findBySQL() was

findBySQL("select catalogue.* from catalogue where Title = 'Whatchadoing?'", 
null)

Here is the exception:

java.lang.IllegalArgumentException: you have placeholders (?) in the query, but 
no arguments are passed
    at org.javalite.activejdbc.DB.find(DB.java:394)
    at org.javalite.activejdbc.LazyList.hydrate(LazyList.java:300)
    at org.javalite.activejdbc.LazyList.listIterator(LazyList.java:610)
    at com.microcinema.microdb.xlsmaker.MetadataReader.lookupObject(MetadataReader.java:949)
....

If I overlooked a note somewhere that null shouldn't be passed to indicate "no 
parameters" then I will fix my code

but in any case, the code in DB.java around line 389 ought to account for 
possible ?s that shouldn't be treated as placeholders for parameters.  My code 
uses findBySQL() this way:

      Method fsql = fieldClass.getDeclaredMethod("findBySQL", args);
      LazyList ps = (LazyList)fsql.invoke(this, sql.toString(), null);

Using findBySQL for such a simple SQL statement obviously is overkill or 
contrary to the intended use of ActiveJDBC, but it was convenient to do so.

Original comment by dr.fra...@gmail.com on 18 Apr 2013 at 12:17

GoogleCodeExporter commented 9 years ago
oh, I get it now. 
The code simply checks if there is a question mark in query, but as in your 
case, it can be part of a string value :)

Any ideas how to fix this?

Original comment by i...@polevoy.org on 19 Apr 2013 at 6:23

GoogleCodeExporter commented 9 years ago
well, I only just started using activejdbc and am unfamiliar with the codebase, 
so I'm not sure how find() gets used internally.  My first thought would be a 
regex based check to count the ?s that are -not- surrounded by quotes.  Not 
immediately sure how to do that in one step.

So count the number of ?s like it does now and then subtract the number of 
matching /["'][^'"]*\?+[^"']*['"]/ patterns. Then that number should match the 
number of param objects.

But then back references ought to be used to match up single quote with single 
quote, etc so SQL like 

field = 'testing"?'

doesn't get matched incorrectly.  I'm not an expert at regex...

Or leave it alone and let the error occur to tell the developer to improve 
their own code and use the features of ActiveJDBC correctly.  :)

Original comment by dr.fra...@gmail.com on 20 Apr 2013 at 9:37

GoogleCodeExporter commented 9 years ago
yeah, I think this can get pretty ugly pretty fast. I think the best thing 
would be to not do any checking and let the DB handle this :)

Original comment by i...@polevoy.org on 21 Apr 2013 at 2:20

GoogleCodeExporter commented 9 years ago

Original comment by i...@polevoy.org on 21 Apr 2013 at 2:24

GoogleCodeExporter commented 9 years ago

Original comment by i...@polevoy.org on 25 Apr 2013 at 5:00