Custom parser for named parameters in prepared statement

Question

I'm creating a small extension to the JDBC API, with the hope of automating some common tasks and avoid boilerplate code.

One of its features will be a basic support for named parameters in prepared statements (that JDBC does not support natively).
Since this is a vital part of the library, I would appreciate your opinions about the module that:

1. Parses the prepared statement, by replacing named parameters with question marks
2. Creates a mapping between the name of the parameter and the index(es) of the resulting question mark(s)

For example, you have:

INSERT INTO customers (name, surname, age) VALUES (:name, :surname, :age)

The parse will produce this string (to be passed to JDBC standard APIs):

INSERT INTO customers (name, surname, age) VALUES (?, ?, ?)

and will also create a mapping between the parameters and the corresponding placeholders, which is:

name -> 1
surname -> 2
age -> 3

This is the whole parsing logic:

import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

public class StatementParser {

    private static final char PARAMETER_PLACEHOLDER = '?';

    private static enum ParseStatus {
        WAITING_FOR_IDENTIFIER, READING_IDENTIFIER
    };

    public static ParseResult parse(String sql, char marker) {

        sql = sql + '\0'; // Insert null character at the end (for parsing)

        ParseStatus status = ParseStatus.WAITING_FOR_IDENTIFIER;

        Map<String, List<Integer>> parameters = new HashMap<String, List<Integer>>();
        StringBuilder resultSql = new StringBuilder();

        StringBuilder currentParameter = new StringBuilder();

        int paramCount = 0;

        for (int offset = 0; offset < sql.length(); offset++) {

            char c = sql.charAt(offset);

            switch (status) {

            case WAITING_FOR_IDENTIFIER:
                if (c == marker) {
                    status = ParseStatus.READING_IDENTIFIER;
                } else {
                    resultSql.append(c);
                }
                break;

            case READING_IDENTIFIER:
                if (c == marker) {
                    throw new StatementParseError(
                            "Unexpected parameter marker at character " + offset);
                } else if (Character.isLetterOrDigit(c)) {
                    currentParameter.append(c);
                } else {
                    int paramNo = ++paramCount;
                    String paramName = currentParameter.toString();
                    if (paramName.isEmpty()) {
                        throw new StatementParseError(
                                "Error parsing parameter #" + paramNo);
                    }
                    if (parameters.containsKey(paramName)) {
                        parameters.get(paramName).add(paramNo);
                    } else {
                        List<Integer> list = new LinkedList<Integer>();
                        list.add(paramNo);
                        parameters.put(paramName, list);
                    }
                    resultSql.append(PARAMETER_PLACEHOLDER);
                    resultSql.append(c);
                    currentParameter.setLength(0); // reset buffer
                    status = ParseStatus.WAITING_FOR_IDENTIFIER;
                }
                break;

            }

        }

        // Remove ending null character
        resultSql.deleteCharAt(resultSql.length() - 1);

        return new ParseResult(resultSql.toString(), parameters);

    }

}

You can easily imagine what is the ParseResult class about, without me posting it here. StatementParseError is a RuntimeException.

My doubts are the following:

1. If a SQL string constant contains the marker character, this parser will not ignore it - whereas it should. Example: SELECT * FROM sometable WHERE foo = :param1 AND bar = 'a:b'. I may try to improve my parser to consider also string delimiters (quotes) and ignore markers inside string constants, but each DBMS has it own rules when it comes, for example, to escape quotes inside strings. Do you think this shortcoming will seriously affect the robustness of my library, or I can just document it and suggest to use another marker as needed?
2. You may have noticed that I add a placeholder \0 character at the end of the string, before parsing it. The reason is that if the input SQL string terminates with the last character of a named parameter identifier, then I need another iteration of the loop to find the end of the parameter identifier and store it in the parameters map. Is there a cleaner way?

Answer 1

If possible, I would completely avoid dealing with raw String for SQL. This is error prone. There are some alternatives in the java world, like hibernate or method chaining (a quick google search reveals jooq). If we stick to your approach, I would get rid of this state machine.

There are several ways to do it.
I will always use this method to handle the map:

private static void insertIntoMap(final Map<String, List<Integer>> mapNameToListPositons, final String name, final int position) {
    final List<Integer> listPositions = mapNameToListPositons.get(name);
    if (listPositions != null)
        listPositions.add(position);
    else
        mapNameToListPositons.put(name, new ArrayList<Integer>(Arrays.asList(position)));
}

---

public static String parse2(final String sqlStatement, final String marker) {
    final Map<String, List<Integer>> mapNameToListPositons = new HashMap<String, List<Integer>>();
    //System.out.println(Arrays.toString(sqlStatement.split("[ (),]")));
    //[INSERT, INTO, customers, , name, , surname, , age, , VALUES, , :name, , :surname, , :age]
    String returnStatement = sqlStatement;
    int parameterPosition = 1;
    for (final String sqlStatementSplitItem : sqlStatement.split("[ (),]")) {
        if (sqlStatementSplitItem.startsWith(marker)) {
            insertIntoMap(mapNameToListPositons, sqlStatementSplitItem.substring(1), parameterPosition);
            ++parameterPosition;
            returnStatement = returnStatement.replace(sqlStatementSplitItem, "?");
        }
    }
    System.out.println(mapNameToListPositons);
    return returnStatement;
}

Here, we use a split. We split for all the values which could be before or after the parametername. You can add more if needed, I took your example as input.
I have added println to make comparisons easier, because I do not know the implementation of your ParseResult class. You can easily change it.

---

public static String parse3(final String sqlStatement, final char marker) {
    final String regexp = "(?<=" + marker + ")\\p{Alnum}+";
    final Map<String, List<Integer>> mapNameToListPositons = new HashMap<String, List<Integer>>();
    final Matcher matcher = Pattern.compile(regexp).matcher(sqlStatement);
    int parameterCount = 1;
    while (matcher.find()) {
        insertIntoMap(mapNameToListPositons, matcher.group(), parameterCount);
        ++parameterCount;
    }
    System.out.println(mapNameToListPositons);
    return sqlStatement.replaceAll(marker + "\\p{Alnum}+", "?");
}

If we use split, we already used a pattern. We can even use a regular expression directly.

But, I normally try to avoid regexp. They are indeed powerful, but nearly always completely unreadable. I do not know any good way to handle them in a readable and maintainable way.

---

public static String parse4(final String sqlStatement, final char marker) {
    final Map<String, List<Integer>> mapNameToListPositons = new HashMap<String, List<Integer>>();
    final StringBuilder resultSqlStatement = new StringBuilder();
    int parameterPosition = 1;

    for (int index = 0; index < sqlStatement.length(); index++) {
        if (sqlStatement.charAt(index) == marker) {
            ++index; //we do not care about the marker. The new index is the first char of the parameter name
            final int parameterStartIndex = index;
            while (Character.isLetterOrDigit(sqlStatement.charAt(index))) //lets search for the end
                ++index;
            insertIntoMap(mapNameToListPositons, sqlStatement.substring(parameterStartIndex, index), parameterPosition);
            ++parameterPosition;
            resultSqlStatement.append(PARAMETER_PLACEHOLDER);
        }
        resultSqlStatement.append(sqlStatement.charAt(index));
    }
    System.out.println(mapNameToListPositons);
    return resultSqlStatement.toString();
}

Here we completely avoid regular expressions and use instead a submethod which is kind of inlined. You could very easily use recursion or a new method, too.

---

All variants have their good and bad points. I would probably use the split or the last one.
Please notice from comparison with your code, that I have added some small changes. I do not list them with quoted code, I will only name them:

• Speaking variable names, no uncommon abbreviations
• You should try to avoid modifying method parameters
• Really the only use case for a linked list is if you plan to do a lot of remove. In all other cases, use ArrayList (we are not talking about multithreading here)
• Try to avoid custom exceptions like StatementParseError. Use IllegalArgumentException, or here IllegalStateException.
• Write unit tests (I did only a basic one, you should add to enhance your confidence in your code
• You should avoid to use the "setLength" method for StringBuilder, instead create a new one. The meaning is not clear and the internals are unknown if they correspond to your expected behavior

I hope, I got everything from my changes. If there are open points, just ask.

Answer 2

Just a quick note: instead the following,

if (parameters.containsKey(paramName)) {
    parameters.get(paramName).add(paramNo);
} else {
    List<Integer> list = new LinkedList<Integer>();
    list.add(paramNo);
    parameters.put(paramName, list);
}

you could use a Multimap.