What is the best-practice for nesting PreparedStatements?

Question

I have several instances where that a section of legacy sql statements is based on a dependency. for example.

if (x !=null)
{
  SQL = "SELECT z WHERE x > y";
}
else
{
  SQL = "SELECT z WHERE x <= y";
} 

SQL2 = SQL + " JOIN a ON b";

I am creating PreparedStatements out of this legacy code. What is the best-practice here. Should I create a PreparedStatement for the var SQL and nest it inside of SQL2 of should there be multiple PreparedStatements based on SQL2 without nesting, or something totlly different?

The code is much more complex than the example, as the SQL var is reused inside many long and complex SQL queries.

EDIT: Project Design requires using PreparedStatements, I don't have the choice of using libraries at this moment.

Answer 1

This is not the proper use of prepared statement parameters. Parameters can be used only in place of a literal value in an SQL expression. Not table names, column names, or other SQL syntax.

You could use some library for building parts of an SQL query. I worked on a library like this in PHP, called Zend_Db_Select.

edit: I googled a bit for a similar library for Java, and I found this option which may be helpful:

• Squiggle is a little Java library for dynamically generating SQL SELECT statements. [Its] sweet spot is for applications that need to build up complicated queries with criteria that changes at runtime. Ordinarily it can be quite painful to figure out how to build this string. Squiggle takes much of this pain away.

It's free and offered under the Apache License, which is a pretty flexible open-source license.

Googling for "java query builder" found a number of other options, but some were not free. Some were visual query builders, not programmatic query builders.

Another option is to use a complicated object-relational mapping framework like Hibernate, but this seems overkill for your current task.

Answer 2

Here is similar question

Short answer - there is no best way. You may end up with something like this:

String selectQuery =
  (new SelectQuery())
  .addColumns(t1Col1, t1Col2, t2Col1)
  .addJoin(SelectQuery.JoinType.INNER_JOIN, joinOfT1AndT2)
  .addOrderings(t1Col1)
  .validate().toString();

But to me it is even worse.

Answer 3

I guess on one hand there's a purist object approach, which is probably not going to be terribly helpful to you if you're trying to make sense of legacy code. I've found that in refactoring really nasty legacy code rather than striving for "perfect" it's often better, and easier, to simplify small pieces, as modular and well documented as you can make them without rewriting the entire app at once. I've found that the biggest hurdle for me in refactoring bad code is that if I take too large steps I can't any longer be confident that I haven't broken anything - if it's that bad, there are probably no unit tests, and there's likely undefined or undocumented behavior.

I'd at least break out the logic and the sql as a first pass. The thing you don't want is something like this:

String sql = "yadda yadda yadda ? yadda yadda WHERE ";
if (mystery condition 1){
   sql = sql + " page=?"
}
else if (mystery condition 2)
{
 sql = sql + " ORDER BY ? "
}

After a while you won't be able to tell what statements are being built. It's not worth saving the bit of duplicating the initial sql. It might be easier to understand as :

private static final String FIND_PAGE_QUERY = "...."
private static final String ORDER_BY_QUERY =" ..."

if (mystery condition 1){
   return process(FIND_PAGE_QUERY, parameters);
}
else if (mystery condition 2)
{
  return process(ORDER_BY_QUERY, parameters);
}

and then just create something to wrap your queries and parameters passed like Spring JDBC Row Mappers or something similar. This is still ugly, but it's easy to do as an incremental step from what you've got, and will at least sort out some of the confusion of query generation.

Answer 4

Ibatis is very good at this.

<select id="queryName" parameterClass="com.blah.X"><!<[CDATA[
  SELECT z
  FROM a
  JOIN b ON a.id = b.foreign_key
  WHERE

  <isNotNull property="value">
    x > y
  </isNotNull>

  <isNull property="value">
    x <= y
  </isNull>

]]></select>

This is but a small fraction of what Ibatis can do but its extremely lightweight. Excellent technology.

Answer 5

>Should I create a PreparedStatement for the var SQL and nest it inside of SQL2

No

>Or should there be multiple PreparedStatements based on SQL2 without nesting

Yes

Furthermore: If you could create one string per query that would be better. I don't really like to mix SQL with code. It makes it harder to debug and to understand, you can't copy/paste to a SQL tool to test it easily. By separating the SQL from your code you'll isolate your query from the operation ( the actual fetch ) and it would be easier to maintain. Plus if the code is not yours it will be a lot easier to understand.

It doesn't matter it looks like your're repeating strings, the point would be to simplify the statements as much as possible.

I would do something like this:

final class DatabaseQueries {
    public final static String SOME_SCENARIO       = "SELECT z WHERE x > y JOIN A, B ";
    public final static String SOME_OTHER_SCENARIO = "SELECT z WHERE x <= y JOIN A, B";
 }

And then use it from your class:

 PreparedStatement pstmt = getCon().prepareStatement( getQuery() );


 private String getQuery() { 
     if( x != null ) { 
          return DatabaseQueries.SOME_SCENARIO;
     } else { 
           return DatabaseQueries.SOME_OTHER_SCENARIO;
     }
 }

While creating the class "DatabaseQueries" you'll find you're repeating a lot of strings, I think it would be fine to susbtitute some part with other constants.

final class DataBaseQueries { 
    // this one is private
    private final static String JOIN_A_B = " join A, B ";
    public final static String SOME_SCENARIO       = "SELECT z WHERE x > y " + JOIN_A_B ;
    public final static String SOME_OTHER_SCENARIO = "SELECT z WHERE x <= y " + JOIN_A_B ;

}

The point here is to make things simpler. This is the first step. In a second step you can create a class to create those queries that are really really complex, but probably YAGNI.

If the queries are too much you can replace it to load them from a ResourceBundle like in this question

I hope this helps.

Answer 6

Another approach would be to move the conditional into the SQL statement itself so you only need one statement. It would be something like:

SELECT z WHERE (? IS NOT NULL AND x > y) OR (? IS NULL AND x <= y)

and then bind an appropriate value for the parameter.

Not sure it's the best approach ... people might find the code less clear. But it is a possibility.