Smajl
Smajl

Reputation: 8015

Effective way of constructing MySQL query in Java

I am developing a portlet in which I access a database quite often. I have to specify the query in a way, that offers the possibility of filtering as a reaction on a user input. The parameters used for filtering are two at the moment, but this number can grow in the future.

At the moment, my construction works pretty well for all inputs, however, I dont think that I am doing it in a right/effective way, since I do not use prepared statement and just construct the query manually.

This is example of my code (serviceFilter is an arrayList and typeFlag is a String)

private String prepareQuery() {
        String query = "SELECT * from messages ";

        // check filters
        if (!typeFlag.equals("ALL")) {
            if (typeFlag.equals("XML")) {
                query += "WHERE type='" + TYPE_XML + "'";
            } else {
                query += "WHERE type='" + TYPE_JAVA + "'";
            }
        }

        // lets see if user specifies some service filtering
        if (serviceFilter.size() > 0) {
            if (!typeFlag.equals("ALL")) {
                query += " AND (";
            } else {
                query += " WHERE (";
            }

            for (int i = 0; i < serviceFilter.size(); i++) {
                if (i>0) {
                    query += " OR ";
                }
                String service = serviceFilter.get(i);
                System.out.println("Filter: " + service);
                query += "sender='" + service + "' OR receiver='" + service + "'";
            }
            query += ")";
        }

        query += " ORDER BY id DESC LIMIT " + String.valueOf(limit);

        System.out.println(query);

        return query;
    }

First problem is, that this has no way to prevent SQL injection (which would not be such a big problem since all the inputs come from checkBoxes and scrollbars, so the user does not actually type anything). I am not sure how to use a prepared statement here, because the population of my arrayList can be quite long and changes for every query.

The query itself, due to this fact can get really long. Here is an example of a query just for two arguments (imagine this for 20 items):

SELECT * from messages  WHERE (sender='GreenServiceESB#GreenListener' OR receiver='GreenServiceESB#GreenListener' OR sender='queue/DeadMessageQueue' OR receiver='queue/DeadMessageQueue') ORDER BY id DESC LIMIT 50

So basically, my question is: Is this an effective way of constructing my query (propably not, right)? What approach would you suggest?

PS: I am using JDBC to connect to db and execute the query, if it is important in any way...

Thanks for any tips!

Upvotes: 1

Views: 194

Answers (2)

Rand
Rand

Reputation: 530

First of all, you hinted at one of your issues - not using a PreparedStatement. Taking user input and using it directly in a SQL statement opens a site to SQL injection attacks.

I think what you're wanting is this:

select * from (
  select *, row_number over (order by id_desc) as rowNum
  from messages
  where sender in (?,?,?,?,?,?,?,?) --8, 16 or however many ?'s you'll need
    or receiver in (?,?,?,?,?,?,?,?)
) results
where rowNum between (1 and ?)
order by rowNum

Now, you bind the parameters with whatever the user input is and if you have extra spots in you IN operator left over, you bind them with some value that can't (or likely won't) be in your table such as null or HiMom#2$@. If you need to support an arbitrary number of values, you run the query multiple times and sort the results in memory yourself.

As far as the row_number function, that may not work in MySQL (I'm not a MySQL guy) but it does have an equivalent (or it could be that limit may be parameterizable, I don't know.

Upvotes: 1

eMko
eMko

Reputation: 1143

If you want to use something like

  create.selectFrom(BOOK)
  .where(PUBLISHED_IN.equal(2011))
  .orderBy(TITLE)

instead of

  SELECT * FROM BOOK
  WHERE PUBLISHED_IN = 2011
  ORDER BY TITLE

you can look to http://www.jooq.org/. It will simplify your code and you can avoid things like "if (something) { sql += " WHERE ..." }". This is antipattern and should not be used when possible.

Upvotes: 1

Related Questions