You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Dan Fabulich <da...@fabulich.com> on 2009/02/09 18:15:44 UTC

[dbutils] QueryRunner.fillStatement and the null problem (long)

This email is long, so here's the executive summary:

Summary: The Oracle JDBC driver is broken in ways that will require some 
Oracle-specific workarounds.  I think I have a good workaround in mind, 
but I don't actually have an Oracle instance available to test.  Can 
somebody tell me if I'm doing something stupid here?

INTRO

QueryRunner has a nice helper method, "fillStatement(PreparedStatement 
stmt, Object[] params)".  It helps you turn calls like this:

   stmt.setString(1, "foo");
   stmt.setInteger(2, 42);
   stmt.setDate(3, myDate);

into calls like this:

   qr.fillStatement(stmt, new Object[] { "foo", new Integer(42), myDate});

In the new "java5" branch, we can use varargs and auto-boxing to make it 
even nicer:

   qr.fillStatement(stmt, "foo", 42, myDate);

Pretty cool, huh?  But it has a problem when you try to pass in a null 
parameter in the object array.

THE PROBLEM WITH NULLS (ORACLE SUCKS)

Under the hood, fillStatement actually does this:

   stmt.setObject(1, "foo");
   stmt.setObject(2, 42);
   stmt.setObject(3, myDate);

But this doesn't work on some databases, including Oracle, when using a 
null parameter.

The Sun documentation says:
http://java.sun.com/javase/6/docs/api/java/sql/PreparedStatement.html#setObject(int,%20java.lang.Object)
> Note: Not all databases allow for a non-typed Null to be sent to the 
> backend. For maximum portability, the setNull or the setObject(int 
> parameterIndex, Object x, int sqlType) method should be used instead of 
> setObject(int parameterIndex, Object x).

That is, you're supposed to pass in a sqlType integer, matching one of the 
defined constants in java.sql.Types.  Types.NULL is a legal constant, but 
it doesn't work with Oracle drivers.

dgraham tried to fix this, using code like this:
http://svn.apache.org/viewvc?view=rev&revision=141728

     for (int i = 0; i < params.length; i++) {
         if (params[i] != null) {
             stmt.setObject(i + 1, params[i]);
         } else {
             // VARCHAR works with many drivers regardless
             // of the actual column type.  Oddly, NULL and
             // OTHER don't work with Oracle's drivers.
             stmt.setNull(i + 1, Types.VARCHAR);
         }
     }

It may well work with "many drivers," but unfortunately it doesn't work 
with PostgreSQL. :-( http://issues.apache.org/jira/browse/DBUTILS-39

Here's a few proposed solutions to this problem:
http://issues.apache.org/jira/browse/DBUTILS-14
http://issues.apache.org/jira/browse/DBUTILS-31
http://issues.apache.org/jira/browse/DBUTILS-41
http://issues.apache.org/jira/browse/DBUTILS-44


PARAMETER META DATA WON'T WORK (ORACLE SUCKS)

DBUTILS-31 suggests an ideal workaround for this problem.  In JDBC 3.0 
(Java 1.4) compliant drivers, you can just ask the PreparedStament what 
the SQL type of any given column is, using the getParameterMetaData() 
statement, like this:

     for (int i = 0; i < params.length; i++) {
         if (params[i] != null) {
             stmt.setObject(i + 1, params[i]);
         } else {
             int sqlType = stmt.getParameterMetaData().getParameterType(i+1);
             stmt.setNull(i + 1, sqlType);
         }
     }

This looks like a pretty good solution to me!  Easy as pie!  But 
unfortunately, it won't work on Oracle!
http://forums.oracle.com/forums/thread.jspa?threadID=585880

You have to register to read that link (it's free but it's a hassle). But 
the most recent post is confirming that it's still broken in Oct 2008. If 
you call getParameterType, you get error ORA-17023 "Unsupported feature." 
Lame!

MY PROPOSED SOLUTION

Francis Townsend replied to DBUTILS-31, suggesting:
> Unfortunately, it does not look as if there is no method that can 
> determine this before calling the getParameterType method. Which means 
> we would need to catch the exception and role back to the previous code, 
> namely use the VARCHAR type when setting null. This would always be 
> thrown by the Oracle driver, severely slowing it down.

That would maybe look something like this:

     for (int i = 0; i < params.length; i++) {
         if (params[i] != null) {
             stmt.setObject(i + 1, params[i]);
         } else {
             int sqlType = Types.VARCHAR;
             try {
                 sqlType = stmt.getParameterMetaData().getParameterType(i+1);
             } catch (Exception e) {}
             stmt.setNull(i + 1, sqlType);
         }
     }

... but that would indeed severely impact Oracle performance.

I think the fix in that case would be to cache the result:

     int sqlType = Types.VARCHAR;
     if (!pmdKnownBroken) {
         try {
             sqlType = stmt.getParameterMetaData().getParameterType(i+1);
         } catch (Exception e) { pmdKnownBroken = true; }
     }
     stmt.setNull(i + 1, sqlType);

Possibly we'd add a new argument to the QueryRunner constructor, allowing 
you to set pmdKnownBroken to true right away.

OPEN QUESTIONS

1) Will caching this result work?  Will it impact thread-safety in some 
negative way?

2) How do we distinguish between getParameterType failing due to an 
out-of-bounds index, vs. failing due to a broken JDBC driver?  Normally we 
could use ParameterMetaData.getParameterCount() ... does that work on 
Oracle?  I don't have an instance available to test.

Thanks for reading all this!  If anyone there has a convenient Oracle 
instance, I'd appreciate help with these questions.

-Dan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [dbutils] QueryRunner.fillStatement and the null problem (long)

Posted by Dan Fabulich <da...@fabulich.com>.
OK, I've checked in a fix for the null problem (and the five bugs 
surrounding it) in revision 742865.  Code review here is welcome.

http://svn.apache.org/viewvc?view=rev&revision=742865

The code passes against Derby, but, as I mentioned earlier, I don't have 
an Oracle test instance to run against, so if some interested party were 
interested in pulling down the code to try it, I'd much appreciate the 
help.

Thanks again!

-Dan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [dbutils] QueryRunner.fillStatement and the null problem (long)

Posted by sebb <se...@gmail.com>.
On 09/02/2009, Dan Fabulich <da...@fabulich.com> wrote:
> Liam Coughlin wrote:
>
>
> > b) Yes it will have a thread safety issue -- you'll need to synchronize on
> > something when setting the pmdKnownBroken variable.
> >
>
>  When setting?  Or when reading?  Remember, once pmdKnownBroken is true, it
> will never again be false, so there's no risk that one thread will try to
> set it to true and another thread will try to set it to false.
>

There are two aspects to thread-safety - mutual exclusion and visibility.

Only the former appears to be addressed here.

The boolean could be made volatile to ensure visibility.

>  IMO the biggest risks here are:
>
>  1) Running getParameterType too many times (ideally we should only ever run
> it once on Oracle, but that may be too much to ask)
>
>  2) We overcompensate and create an unecessary synchronization bottleneck
>
>  What if I just make pmdKnownBroken "volatile"?
>
>
>  -Dan
>
> ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [dbutils] QueryRunner.fillStatement and the null problem (long)

Posted by Dan Fabulich <da...@fabulich.com>.
Liam Coughlin wrote:

> b) Yes it will have a thread safety issue -- you'll need to synchronize on
> something when setting the pmdKnownBroken variable.

When setting?  Or when reading?  Remember, once pmdKnownBroken is true, it 
will never again be false, so there's no risk that one thread will try to 
set it to true and another thread will try to set it to false.

IMO the biggest risks here are:

1) Running getParameterType too many times (ideally we should only ever 
run it once on Oracle, but that may be too much to ask)

2) We overcompensate and create an unecessary synchronization bottleneck

What if I just make pmdKnownBroken "volatile"?

-Dan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [dbutils] QueryRunner.fillStatement and the null problem (long)

Posted by Liam Coughlin <ls...@gmail.com>.
snip

>
>    int sqlType = Types.VARCHAR;
>    if (!pmdKnownBroken) {
>        try {
>            sqlType = stmt.getParameterMetaData().getParameterType(i+1);
>        } catch (Exception e) { pmdKnownBroken = true; }
>    }
>    stmt.setNull(i + 1, sqlType);
>
> Possibly we'd add a new argument to the QueryRunner constructor, allowing
> you to set pmdKnownBroken to true right away.
>
/snip

IMHO -- this is a good solution.  Taking advantage of JDBC 3.0 seems like it
would be the right direction to move in rather then implementing spotty
parameter type filler classes.



>
> OPEN QUESTIONS
>
> 1) Will caching this result work?  Will it impact thread-safety in some
> negative way?
>

a)  Yes it should work.
b) Yes it will have a thread safety issue -- you'll need to synchronize on
something when setting the pmdKnownBroken variable.


>
> 2) How do we distinguish between getParameterType failing due to an
> out-of-bounds index, vs. failing due to a broken JDBC driver?  Normally we
> could use ParameterMetaData.getParameterCount() ... does that work on
> Oracle?  I don't have an instance available to test.
>

ParameterMetaData.getParameterCount() works fine, and it works fine on
oracle a well.

Thanks for taking the time to summarize and propose a solution -- hopefully
someone will pick up on your work!

Cheers,
-L