Reputation: 8015
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
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
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