Run arbitrary INSERT INTO SQL Query using parameters While DoCmd.RunSQL is all well and good for simple code, the number of times I've run into problems with unescaped apostrophes and the like was starting to nark a bit. With that in mind I crafted the below, a first attempt at a SQL wrapper for VBA which constructs and executes an arbitrary INSERT INTO query using parameters.



'Fields should be a comma-delimited string
Public Sub InsertInto(TableName As String, Fields As String, Values As Variant)
    Dim qdf As DAO.QueryDef
    Dim strParams As String
    Dim i As Long
    Dim strSQL As String

On Error GoTo Err_InsertInto
    'Assigns strParams value in format "[p1],[p2],[p3],...,[pN]"
    For i = 0 To UBound(Values)
        strParams = strParams &amp; "[p" &amp; i &amp; "],"
    Next i
    strParams = Left(strParams, Len(strParams) - 1) 'Strip trailing comma

    'Construct SQL
    strSQL = "INSERT INTO " &amp; TableName &amp; "(" &amp; Fields &amp; ") " &amp; _
             "VALUES (" &amp; strParams &amp; ");"
    Set qdf = CurrentDb.CreateQueryDef("", strSQL)

    'Give values to parameters
    For i = 0 To UBound(Values)
        qdf.Parameters("[p" &amp; i &amp; "]") = Values(i)
    Next i

    qdf.Execute

Exit_InsertInto:
    Set qdf = Nothing
    Exit Sub

Err_InsertInto:
    Select Case Err.Number
        Case 13   'Type mismatch i.e. Values is not an array
            Values = Array(Values)
            Resume
        Case Else 'Unhandled Error
            Set qdf = Nothing 'Release QueryDef object

            'Error is unhandled here, force it to 'bubble up' to calling code
            Dim iErrNum As Long, strErrDesc As String
            iErrNum = Err.Number
            strErrDesc = Err.Description
            On Error GoTo 0
            Err.Raise iErrNum, "SQLOps.InsertInto", strErrDesc
    End Select
End Sub


Likely to be called in a manner similar to (this example was used as a successful test case):

InsertInto "tblErrorLog", _
"ErrorNum,Description,User,When,Origin", _
Array(10,"Error's Description",Environ("USERNAME"),Now(),"A Ghost")


So far it seems to be working fine and does indeed alleviate the issues with unescaped apostrophes but I'm wondering whether this can be improved so I could use it as a sort of template for crafting similar functions for other SQL operations e.g. UPDATE SET / SELECT FROM.

Can this code be improved and are there any unforeseen bugs I should watch out for?
This code can't handle fields with spaces in them unless you specifically pass them pre-wrapped in square brackets. Things such as My Field would need to go into your parameter as "field1, [My odd field], field3". So, a definite area for improvement would be to handle that corner case.

I also don't really care for the comma delimited string parameter. I feel that it would be much cleaner and nicer to pass in a string array representing the field names. So, something along the lines of this psuedo code might be a bit cleaner.

Dim fields As Variant
fields = Array("field1","My odd field","field3")

InsertInto "Table1", fields, values)

' ...

Public Sub InsertInto(...)
    AddBracketsToStrings fields

    '...

End Sub




The next thing I notice is these comments:


'Assigns strParams value in format "[p1],[p2],[p3],...,[pN]"
...
'Construct SQL
...
'Give values to parameters
...



The comments are nice, and add a bit of clarity to the code, but these kinds of comments indicate an opportunity to extract subroutines/functions. Each of these comments could be replaced with a well named function.

ConvertValuesToParameters values
Set qdf = CurrentDb.CreateQueryDef("", ConstructSQL(TableName, Fields, strParams))
SetParameterValues qdf, values


Which brings me to the SQL Injection issue. It's never a good idea to concatenate SQL strings together. It's much better to use parameterized queries. This may require that you switch to using ADODB instead of DAO. I'm not sure. I don't know if DAO supports parameterized queries.




Case 13   'Type mismatch i.e. Values is not an array



That's a good comment, but a constant value would be better.

Const TypeMismatchError As Long = 13

Select Case Err.Number
    Case TypeMisMatchError 
        ' ...