SQL code inside Java classes

Question

Our current project does not use Hibernate (for various reasons) and we are using Spring's SimpleJdbc support to perform all our DB operations. We have a utility class that abstracts all CRUD operations but complex operations are performed using custom SQL queries.

Currently our queries are stored as String constants inside the service classes themselves and are fed to a utility to be execute by the SimpleJdbcTemplate. We are at an impasse where readability has to be balanced with maintainability. SQL code inside the class itself is more maintainable since it resides with the code that uses it. On the other hand if we store these queries in an external file (flat or XML) the SQL itself would be more readable as compared to escaped java string syntax.

Has anyone encountered a similar problem? What is a good balance? Where do you keep your custom SQL in your project?

A sample query is as follows:

private static final String FIND_ALL_BY_CHEAPEST_AND_PRODUCT_IDS = 
"    FROM PRODUCT_SKU T \n" +
"    JOIN \n" +
"    ( \n" +
"        SELECT S.PRODUCT_ID, \n" +
"               MIN(S.ID) as minimum_id_for_price \n" +
"          FROM PRODUCT_SKU S \n" +
"         WHERE S.PRODUCT_ID IN (:productIds) \n" +
"      GROUP BY S.PRODUCT_ID, S.SALE_PRICE \n" +
"    ) FI ON (FI.PRODUCT_ID = T.PRODUCT_ID AND FI.minimum_id_for_price = T.ID) \n" +
"    JOIN \n" +
"    ( \n" +
"        SELECT S.PRODUCT_ID, \n" +
"               MIN(S.SALE_PRICE) as minimum_price_for_product \n" +
"          FROM PRODUCT_SKU S \n" +
"         WHERE S.PRODUCT_ID IN (:productIds) \n" +
"      GROUP BY S.PRODUCT_ID \n" +
"    ) FP ON (FP.PRODUCT_ID = T.PRODUCT_ID AND FP.minimum_price_for_product = T.sale_price) \n" +
"WHERE T.PRODUCT_ID IN (:productIds)";

This is how it would look like in a flat SQL file:

--namedQuery: FIND_ALL_BY_CHEAPEST_AND_PRODUCT_IDS
FROM PRODUCT_SKU T 
JOIN 
( 
    SELECT S.PRODUCT_ID, 
           MIN(S.ID) as minimum_id_for_price 
      FROM PRODUCT_SKU S 
     WHERE S.PRODUCT_ID IN (:productIds) 
  GROUP BY S.PRODUCT_ID, S.SALE_PRICE 
) FI ON (FI.PRODUCT_ID = T.PRODUCT_ID AND FI.minimum_id_for_price = T.ID) 
JOIN 
( 
    SELECT S.PRODUCT_ID, 
           MIN(S.SALE_PRICE) as minimum_price_for_product 
      FROM PRODUCT_SKU S 
     WHERE S.PRODUCT_ID IN (:productIds) 
  GROUP BY S.PRODUCT_ID 
) FP ON (FP.PRODUCT_ID = T.PRODUCT_ID AND FP.minimum_price_for_product = T.sale_price) 
WHERE T.PRODUCT_ID IN (:productIds)

Answer 1

I would imagine that storing a query in an external file, and then have the application read it when needed presents a HUGE security hole.

What happens if an evil mastermind has access to that file and changes your query?

For example changes

 select a from A_TABLE;

TO

 drop table A_TABLE;

OR

 update T_ACCOUNT set amount = 1000000

Plus, it adds the complexity of having to mantain two things: Java Code, and SQL query files.

EDIT: Yes, you can change your queries without recompiling your app. I don't see the big deal there. You could recompile classes that hold/create sqlQueries only, if the project is too big. Besides, if documentation is poor, maybe you'd end up changing the incorrect file, and that will turn into an enormous silent bug. No exception or error codes will be thrown, and when you realize what you've done, it may be too late.

A different approach would be to have some sort of SQLQueryFactory, well documented, and with methods that return a SQL query that you want to use.

For example

public String findCheapest (String tableName){

      //returns query.
}

Answer 2

Why not use Stored Procedures instead of hard coding queries? Stored Procs will increase maintainability and provide more security for things like SQL Interjection Attacks.

Answer 3

In the class is probably best - If the queries are long enough that the escaping is a problem you probably want to be looking at either stored procedures or simplifying the query.

Answer 4

I've run into this as well, currently for the same reason - a project based on spring jdbc. My experience is that while it's not great to have the logic in the sql itself, there's really no better place for it , and putting in the application code is slower than having the db do it and not necessarily any clearer.

Biggest pitfalls I've seen with this is where the sql starts to proliferate all over the project, with multiple variations. "Get A, B, C from FOO". "Get A, B, C, E from Foo", etc.etc. This sort of proliferation is especially likely as the project hits a certain critical mass - it may not seem like an issue with 10 queries, but when there's 500 queries scattered throughout the project it becomes much harder to figure out if you've already done something. Abstracting out the basic CRUD operations puts you way ahead of the game here.

Best solution, AFAIK, is to be rigorously consistent with the coded SQL - commented, tested, and in a consistent place. Our project has 50-line uncommented sql queries. What do they mean? Who knows?

As for queries in external files, I don't see what this buys - You're still just as reliant on the SQL, and with the exception of the (questionable) aesthetic improvement of keeping the sql out of the classes, your classes are still just as reliant on the sql -.e.g you generally separate resources to get the flexibility to plug-in replacement resources, but you couldn't plug in replacement sql queries as that'd change the semantic meaning of the class or not work at all. So it's an illusory code-cleanliness.

Answer 5

One option is to use iBatis. It's fairly lightweight compared to a full-blown ORM like Hibernate, but provides a means to store your SQL queries outside your .java files

Answer 6

We store all our SQL in a class as a bunch of static final strings. For readability we spread it over a few lines concatenated using +. Also, I am not sure if you need to escape any thing - "Strings" are enclosed in single quotes in sql.

Answer 7

We had a project where we used the exact approach you are, except we externalized each query to a separate text file. Each file was read in (once) using Spring's ResourceLoader framework and the application worked via an interface like this:

public interface SqlResourceLoader {
    String loadSql(String resourcePath);
}

A distinct advantage with this was that having the SQL in an un-escaped format allowed for easier debugging -- just read the file into a query tool. Once you have more than a few queries of moderate complexity, dealing with un/escaping to/from the code while testing & debugging (particularly for tuning) it was invaluable.

We also had to support a couple of different databases, so it allowed for swapping the platform more easily.

Answer 8

Based on the issue as you explained it, there is no real choice here except to keep it in the code, and get rid of the /n characters everywhere. This is the only thing that you mentioned as affecting readability and they are absolutely unnecessary.

Unless you have other issues with it being in the code your problem is easily solved.

Answer 9

A fairly radical solution would be to use Groovy to specify your queries. Groovy has language-level support for multi-line strings and string interpolation (amusingly known as GStrings).

For example using Groovy, the query you've specified above would simply be:

class Queries
    private static final String PRODUCT_IDS_PARAM = ":productIds"

    public static final String FIND_ALL_BY_CHEAPEST_AND_PRODUCT_IDS = 
    """    FROM PRODUCT_SKU T 
        JOIN 
        ( 
            SELECT S.PRODUCT_ID, 
                   MIN(S.ID) as minimum_id_for_price 
              FROM PRODUCT_SKU S 
             WHERE S.PRODUCT_ID IN ($PRODUCT_IDS_PARAM) 
          GROUP BY S.PRODUCT_ID, S.SALE_PRICE 
        ) FI ON (FI.PRODUCT_ID = T.PRODUCT_ID AND FI.minimum_id_for_price = T.ID) 
        JOIN 
        ( 
            SELECT S.PRODUCT_ID, 
                   MIN(S.SALE_PRICE) as minimum_price_for_product 
              FROM PRODUCT_SKU S 
             WHERE S.PRODUCT_ID IN ($PRODUCT_IDS_PARAM) 
          GROUP BY S.PRODUCT_ID 
        ) FP ON (FP.PRODUCT_ID = T.PRODUCT_ID AND FP.minimum_price_for_product = T.sale_price) 
    WHERE T.PRODUCT_ID IN ($PRODUCT_IDS_PARAM) """

You can access this class from Java code, just like you would as if it were defined in Java, e.g.

String query = QueryFactory.FIND_ALL_BY_CHEAPEST_AND_PRODUCT_IDS;

I'll admit that adding Groovy to your classpath just to make your SQL queries look nicer is a bit of a "sledgehammer to crack a nut" solution, but if you're using Spring, there's a fair chance you already Groovy on your classpath.

Also, there's likely a lot of other places in your project where you could use Groovy (instead of Java) to improve your code, (particularly now that Groovy is owned by Spring). Examples include writing test cases, or replacing Java beans with Groovy beans.

Answer 10

I've stored SQL as both Strings inside a Java class and as separate files that were loaded at run time. I greatly preferred the latter for two reasons. First, the code is more readable by a wide margin. Second, it's easier to test the SQL in isolation if you store it in a separate file. In addition to that, it was easier to get someone better than me at SQL to help me with my queries when they were in separate files.

Answer 11

What you need here is SQLJ which is a SQL Java-Preprocessor. Sadly, apparently it never took off, although I have seen some IBM and Oracle implementations. But they are quite outdated.

If I were you and had lots and lots of queries on the system, I would stored them in a separate file and load them at runtime.

Answer 12

We use stored procedures. This is good for us because we use Oracle Fine Grain Access. This allows us to restrict a user from seeing a particular report or search results by limiting their access to the relevant procedure. It also gives us a little bit of a performance boost.

Answer 13

From my experience it is better to leave the SQL statements in the code not separating them makes things more maintainable (like annotations vs. config files), but now I have a debate with a team member about it.

Answer 14

I have a small program that access clipboard and escape/unscape plain text with Java string literals.

I have it as a shortcut on the "quick launch" tool bar so the only thing I need to do is

Ctrl+C, Click jar, Ctrl+V

Either when I want to run my "code" into SQL tool, or vice versa.

So I usually have something like this:

String query = 
    "SELECT a.fieldOne, b.fieldTwo \n"+
    "FROM  TABLE_A a, TABLE b \n"+ 
    "... etc. etc. etc";


logger.info("Executing  " + query  );

PreparedStatement pstmt = connection.prepareStatement( query );
....etc.

Which gets transformed into:

    SELECT a.fieldOne, b.fieldTwo 
    FROM  TABLE_A a, TABLE b
    ... etc. etc. etc

Either because at some projects I cannot create separate files, or because I'm paranoiac and I feel some bits will get inserted/deleted while reading from the external file ( usually a invisible \n that makes

select a,b,c 
from

into

select a,b,cfrom

IntelliJ idea does the same for you automagically but just from plain to code.

Here's an old version I recovered. Its a bit broken and does not handle ?.

Let me know if someone improves it.

import java.awt.Toolkit;
import java.awt.datatransfer.Clipboard;
import java.awt.datatransfer.DataFlavor;
import java.awt.datatransfer.ClipboardOwner;
import java.awt.datatransfer.Transferable;
import java.awt.datatransfer.StringSelection;
import java.awt.datatransfer.UnsupportedFlavorException;
import java.io.IOException;

/**
 * Transforms a plain string from the clipboard into a Java 
 * String literal and viceversa.
 * @author <a href="http://stackoverflow.com/users/20654/oscar-reyes"&gt;Oscar Reyes</a>
 */
public class ClipUtil{

    public static void main( String [] args ) 
                                throws UnsupportedFlavorException,
                                                     IOException {

        // Get clipboard
        Toolkit toolkit = Toolkit.getDefaultToolkit();
        Clipboard clipboard = toolkit.getSystemClipboard();

        // get current content.
        Transferable transferable = clipboard.getContents( new Object() ); 
        String s = ( String ) transferable.getTransferData( 
                                                DataFlavor.stringFlavor );

        // process the content
        String result = process( s );

        // set the result
        StringSelection ss = new StringSelection( result );
        clipboard.setContents( ss, ss );

    }
    /**
     * Transforms the given string into a Java string literal 
     * if it represents plain text and viceversa. 
     */
    private static String process( String  s ){
        if( s.matches( "(?s)^\\s*\\\".*\\\"\\s*;$" ) ) {
            return    s.replaceAll("\\\\n\\\"\\s*[+]\n\\s*\\\"","\n")
                       .replaceAll("^\\s*\\\"","")
                       .replaceAll("\\\"\\s*;$","");
        }else{
            return     s.replaceAll("\n","\\\\n\\\" +\n \\\" ")
                        .replaceAll("^"," \\\"")
                        .replaceAll("$"," \\\";");
        }
    }
}

Answer 15

Since you already use Spring why not put the SQL in the Spring config file and DI it into the DAO class? That is a simple way to externalize the SQL string.

HTH Tom

Answer 16

Oscar, I've done the following to code to better understand the regex:

    private static String process(String s) {
 String newlineToken = "\n";
 String eolToken = "\r\n";

 if (s.matches("(?s)^\\s*\\\".*\\\"\\s*;$")) {
  return s.replaceAll("\\\\n\\\"\\s*[+]\n\\s*\\\"", "\n").replaceAll("^\\s*\\\"", "").replaceAll("\\\"\\s*;$", "");
 } else {
  System.out.println("Replaced plain text with java string literal");
  //replaces each eol token with a newline character, then inserts a newline (for the next line)
  //and adds a quote
  String string = s.replaceAll(newlineToken, \\\\n\" +\n \" );
  //replaces start of first line with a quote
  string = string.replaceAll("^", "\"");
  //replaces end of line with a quote and semi-colon
  string = string.replaceAll("$", " \";");
  System.out.println("Done.");
  return string;
 }
}

but I'm confused about the

\\\\n\" +\n \"

what is the 'plus' doing there? I notice that the result is better (without it you get an extra line) but can't tell how it works as I thought that it was just replacement text, but it actually seems to be doing some work too. Can you explain?

PS. I wanted to respond directly to your post, but didn't see how...

Answer 17

I prefer the external option. I support multiple projects and find it much more difficult to support internal SQL because you have to compile and redeploy each time you want to make a slight change to the SQL. Having the SQL in an external file allows you to make large and small changes easily with less risk. If you're just editing the SQL, there's no chance of putting in a typo that breaks the class.

For Java, I use the Properties class which represents the SQL in a .properties file, which allows you to pass the SQL queries around if you want to re-use the queries rather than read the file in multiple times.