views:

264

answers:

7

This is a Query in VBA (Access 2007) I have 3 strings defined:

str_a = "db.col1 = 5"
str_b = " and db.col2 = 123"
str_c = " and db.col3 = 42"

Then I use these in the WHERE part of my Query:

"WHERE '" & str_a & "' '" & str_b & "' '" & str_c & "' ;"

This fails, but If I paste in the strings like this:

"WHERE db.col1 = 5 and db.col2 = 123 and db.col3 = 42;"

It works perfectly. I'm guessing the syntax is wrong when using multiple variables in a string.
Anyone have any hints?

+3  A: 

You've got some extra single quotes in there.

Try this:

"WHERE " & str_a & str_b  & str_c

Note: In general, you shouldn't build query strings by concatenating strings, because this leaves you vulnerable to SQL injection, and mishandles special characters. A better solution is to use prepared statements. But assuming you're operating in a very controlled environment the solution I gave should work.

recursive
No this gave an error, I think the the '" are neccessary, from what I've seen online.
Nick S.
What error. And no, they are definitely not necessary.
recursive
thegreekness: I am fairly willing to bet that your table is not called db, at least, I hope it is not. Get rid of db. and see if you still get an error.
Remou
No its not the name of my table, i just put that for the sake of this post, put I am using the actual name of my table.
Nick S.
+2  A: 

Quick tip about troubleshooting SQL that is built dynamically: echo the SQL string resulting from all the concatenation and interpolation, instead of staring at your code.

WHERE 'db.col1 = 5' ' and db.col2 = 123' ' and db.col3 = 42';

Nine times out of ten, the problem becomes a lot more clear.

Bill Karwin
Do you know how I can do this in VBA?
Nick S.
Just put your SQL string into a variable and use Wscript.echo or Cscript.echo.
Bill Karwin
Use Debug.Print, wscript will not suit in VBA.
Remou
See also my comment on the post by recursive.
Remou
+2  A: 
"WHERE '" & str_a & "' '" & str_b & "' '" & str_c & "' ;"

will include single quotes within your completed WHERE clause. And the working version you showed us has none:

"WHERE db.col1 = 5 and db.col2 = 123 and db.col3 = 42;"

So, try constructing the WHERE clause with no single quotes:

"WHERE " & str_a & " " & str_b & " " & str_c & ";"

For debugging purposes, it's useful to view the final string after VBA has finished constructing it. If you're storing the string in a variable named strSQL, you can use:

Debug.Print strSQL

to display the finished string in the Immediate Window of the VBE editor. (You can get to the Immediate Window with the CTRL+g keyboard shortcut.

Alternatively, you could display the finished string in a message box window:

MsgBox strSQL

Good luck, Hans

HansUp
I think this will work, I think what you have here looks correct but I will have to wait til Monday when I get back to work to try it. Thanks for the help the debug will be very useful. I'm just not that familiar with VBA
Nick S.
A: 

Some comments on constructing WHERE clauses in VBA.

Your example is by definition going to be incorrect, because you're putting single quotes where they aren't needed. This:

str_a = "db.col1 = 5"
str_b = " and db.col2 = 123"
str_c = " and db.col3 = 42"
"WHERE '" & str_a & "' '" & str_b & "' '" & str_c & "' ;"

...will produce this result:

WHERE 'db.col1 = 5' ' and db.col2 = 123' ' and db.col3 = 42' ;

This is obviously not going to work.

Take the single quotes out and it should work.

Now, that said, I'd never do it that way. I'd never put the AND in the substrings that are used to construct the WHERE clause, because what would I do if I have a value for the second string but not for the first?

When you have to concatenate a number of strings with a delimiter and some can be unassigned, one thing to do is to just concatenate them all and not worry if the string before the concatenation is unassigned of not:

str_a = "db.col1 = 5"
str_b = "db.col2 = 123"
str_c = "db.col3 = 42"

To concatenate that, you'd do:

If Len(str_a) > 0 Then
   strWhere = strWhere & " AND " str_a
End If
If Len(str_b) > 0 Then
   strWhere = strWhere & " AND " str_b
End If
If Len(str_c) > 0 Then
   strWhere = strWhere & " AND " str_c
End If

When all three strings are assigned, that would give you:

" AND db.col1 = 5 AND db.col2 = 123 AND db.col3 = 42"

Just use Mid() to chop of the first 5 characters and it will always come out correct, regardless of which of the variables have values assigned:

strWhere = Mid(strWhere, 6)

If none of them are assigned, you'll get a zero-length string, which is what you want. If any one of them is assigned, you'll first get " AND ...", which is an erroneous leading operator, which you just chop out with the Mid() command. This works because you know that all the results before the Mid() will start with " AND " no matter what -- no needless tests for whether or not strWhere already has been assigned a value -- just stick the AND in there and chop it off at the end.

On another note, someone mentioned SQL injection. In regards to Access, there was a lengthy discussion of that which considers a lot of issues close to this thread:

Non-Web SQL Injection

David-W-Fenton
This is really helpful I will try to use something like this when I get back to work on Monday.
Nick S.
Also the reason I'm doing this is because ealrier in the code I have it set to require the first string to be present, then there are some if else where the last 2 strings could just be "" I know this doesnt seem safe but its a very controlled environment
Nick S.
If you get a zero length string, you'll need to get rid of 'WHERE'
Jeff O
There is no "WHERE" stored in any of my strings in my example. Obviously, after strWhere = Mid(strWhere, 6), you'd check if Len(strWhere)>0 and only then concatenate with "WHERE ".
David-W-Fenton
+2  A: 

For VB6/VBA dynamic SQL, I always find it more readable to create an SQL template, and then use the Replace() function to add in the dynamic parts. Try this out:

Dim sql As String
Dim condition1 As String
Dim condition2 As String
Dim condition3 As String

sql = "SELECT db.col1, db.col2, db.col3 FROM db WHERE <condition1> AND <condition2> AND <condition3>;"

condition1 = "db.col1 = 5"
condition2 = "db.col2 = 123"
condition3 = "db.col3 = 'ABCXYZ'"

sql = Replace(sql, "<condition1>", condition1)
sql = Replace(sql, "<condition2>", condition2)
sql = Replace(sql, "<condition3>", condition3)

However, in this case, the values in the WHERE clause would change, not the fields themselves, so you could rewrite this as:

Dim sql As String

sql = "SELECT col1, col2, col3 FROM db "
sql = sql & "WHERE col1 = <condition1> AND col2 = <condition2> AND col3 = '<condition3>';"

sql = Replace(sql, "<condition1>", txtCol1.Text)
sql = Replace(sql, "<condition2>", txtCol2.Text)
sql = Replace(sql, "<condition3>", txtCol3.Text)
HardCode
clearly more readable!
Philippe Grondier
This is actually very close to what I'm actually doing I just didn't include this in the post. I really appreciate the help I might change what I have to resemble this more closely.
Nick S.
Edited to fix type and show another method.
HardCode
The only problem with this method for me is that depending on a previous selection by the user condition3 might be "" that way the user could see more results, so your method would return an error bc there would be an extra 'AND' in the Query string
Nick S.
Well, naturally you would have to evolve the query into something useful. But you could still build a dynamic string variable template and use the Replace() method. Possibilities are limitless ;)
HardCode
A: 

I have my favorite "addANDclause" function, with the following parameters:

public addANDclause( _ 
    m_originalQuery as string, _
    m_newClause as string) _
as string
  • if m_originalQuery doe not contains the WHERE keyword then addANDClause() will return the original query with a " WHERE " added to it.
  • if m_orginalQuery already contains the WHERE keyword then addANDClause() will return the original query with a " AND " added to it.

So I can add as many "AND" clauses as possible. With your example, I would write the following to create my SQL query on the fly:

m_SQLquery = "SELECT db.* FROM db"
m_SQLquery = addANDClause(m_SQLQuery, "db.col1 = 5")
m_SQLQuery = addANDClause(m_SQLQuery, "db.col2 = 123")
m_SQLQuery = addANDClause(m_SQLQuery, "db.col3 = 42")

Of course, instead of these fixed values, such a function can pick up values available in bound or unbound form controls to build recordset filters on the fly. It is also possible to send parameters such as:

m_SQLQuery = addANDClause(m_SQLQuery, "db.text1 like 'abc*'")
Philippe Grondier
A: 

While dynamic SQL can be more efficient for the engine, some of the comments here seem to endorse my view that dynamic SQL can be confusing to the human reader, especially when they didn't write the code (think of the person who will inherit your code).

I prefer static SQL in a PROCEDURE and make the call to the proc dynamic at runtime by choosing appropriate values; if you use SQL DDL (example below) to define the proc you can specify DEFAULT values (e.g. NULL) for the parameters so the caller can simply omit the ones that are not needed e.g. see if you can follow the logic in this proc:

CREATE PROCEDURE MyProc
(
   arg_col1 INTEGER = NULL, 
   arg_col2 INTEGER = NULL, 
   arg_col3 INTEGER = NULL
)
AS 
SELECT col1, col2, col3
  FROM db 
 WHERE col1 = IIF(arg_col1 IS NULL, col1, arg_col1) 
       AND col2 = IIF(arg_col2 IS NULL, col2, arg_col2) 
       AND col3 = IIF(arg_col3 IS NULL, col3, arg_col3);

Sure, it may not yield the best execution plan but IMO you have to balance optimization against good design principles (and it runs really quick on my machine :)

onedaywhen