1

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?

HansUp
  • 95,961
  • 11
  • 77
  • 135
Nick Sinas
  • 2,594
  • 11
  • 42
  • 51
  • When you write MsgBox(myQuery) do you get EXACTLY what your pasted example has? If not, that's your problem. – Jonathan Allen Mar 27 '09 at 22:12
  • Is that the actual code that you are using? If it is, then recursive's suggestion should work. If it's not, then paste the actual code, it's very hard to find an error by looking at some code where the error is not... – Guffa Mar 27 '09 at 22:59
  • before sending your SQL request on the server, please add a debug.print once your string is built. The syntax error will then be obvious – Philippe Grondier Mar 28 '09 at 17:11

7 Answers7

3

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
  • 83,943
  • 34
  • 151
  • 241
  • No this gave an error, I think the the '" are neccessary, from what I've seen online. – Nick Sinas Mar 27 '09 at 22:01
  • 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. – Fionnuala Mar 28 '09 at 02:23
  • 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 Sinas Mar 28 '09 at 16:45
3
"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 VB 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
HansUp
  • 95,961
  • 11
  • 77
  • 135
  • 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 Sinas Mar 28 '09 at 16:56
2

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
  • 538,548
  • 86
  • 673
  • 828
2

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
  • 6,497
  • 4
  • 31
  • 54
  • 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 Sinas Mar 28 '09 at 16:54
  • 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 Sinas Mar 29 '09 at 00:20
  • 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 Mar 30 '09 at 16:34
0

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

Community
  • 1
  • 1
David-W-Fenton
  • 22,871
  • 4
  • 45
  • 58
  • This is really helpful I will try to use something like this when I get back to work on Monday. – Nick Sinas Mar 28 '09 at 16:50
  • 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 Sinas Mar 28 '09 at 16:52
  • If you get a zero length string, you'll need to get rid of 'WHERE' – JeffO Mar 30 '09 at 18:13
  • 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 Mar 31 '09 at 17:34
0

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
  • 10,900
  • 3
  • 33
  • 72
0

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
  • 55,269
  • 12
  • 100
  • 138