VBA Query returning no values

Question

I am trying to connect to an SQL Server and pull certain data with a certain date range, so that the user just adds the year they want to pull and it will get all the data for that year.

The query works in SQL, but as soon as I add it to VBA it pulls nothing. Can someone please help or explain to me why?

At the moment I can connect to the database and my data record works because if I use a smaller query it works fine.

Option Explicit

Sub ADOExcelSQLServer()

    Dim conn As ADODB.Connection

    Dim Server_Name As String
    Dim Database_Name As String
    Dim User_ID As String
    Dim Password As String

    Dim Data As ADODB.Recordset

    Server_Name = "******" ' Enter your server name here
    Database_Name = "******" ' Enter your database name here
    User_ID = "*****" ' enter your user ID here
    Password = "*****" ' Enter your password here

    Set conn = New ADODB.Connection
    Set Data = New ADODB.Recordset

    conn.ConnectionString = "Provider=SQLNCLI10;Server=" & Server_Name & ";Database=" & Database_Name & ";Uid=" & User_ID & ";Pwd=" & Password & ";"

    conn.Open

    On Error GoTo CloseConnection

    With Data
    .ActiveConnection = conn
    .Source = GetYearString
    .LockType = adLockReadOnly
    .CursorType = adOpenForwardOnly
    .Open
    End With

    Sheets("Sheet3").Range("D4:O4").CopyFromRecordset Data

    On Error GoTo 0

    Data.Close

CloseConnection:
    conn.Close

End Sub

Function GetYearString() As String
    Dim Year As Integer
    Dim SQLString As String
    Year = Application.InputBox("Enter the Year of choice ?", Type:=1)
    SQLString = "DECLARE @Test TABLE"
    SQLString = SQLString & "("
    SQLString = SQLString & "ID INT IDENTITY(1,1),"
    SQLString = SQLString & "Value Float"
    SQLString = SQLString & ")"
    SQLString = SQLString & "DECLARE @InputDate DATETIME"
    SQLString = SQLString & "SET @InputDate = '" & Year & "-01-01'"
    SQLString = SQLString & "WHILE @InputDate <= CAST('" & Year & "-12-01' AS DATETIME)"
    SQLString = SQLString & "BEGIN"
    SQLString = SQLString & "DECLARE @MonthStartDate DATETIME"
    SQLString = SQLString & "SELECT @MonthStartDate = CAST(DATEADD(dd, - DATEPART(dd, @InputDate) + 1, @InputDate)AS DATETIME)"
    SQLString = SQLString & "INSERT INTO @Test"
    SQLString = SQLString & "SELECT MAX([Value])*2 FROM DataLog2"
    SQLString = SQLString & "WHERE DateAdd(HOUR,2,TimestampUTC) >= @MonthStartDate AND DateAdd(HOUR,2,TimestampUTC) < DATEADD(DAY,1,@MonthStartDate) AND SourceID = 26 AND quantityid = 129"
    SQLString = SQLString & "SET @InputDate = DATEADD(MONTH, 1, @InputDate)"
    SQLString = SQLString & "End"
    SQLString = SQLString & "SELECT Value FROM @Test"
    GetYearString = SQLString
End Function

There may be some kind of error in the SQL statement. Check the details of the [Err](https://msdn.microsoft.com/en-us/library/office/gg251525.aspx) object within the `CloseConnection` label. — Zev Spitz, Oct 21 '15 at 12:51
Also keep in mind that you are concatenating a string without any spaces whatsoever. You'll end up with syntax like this: `BEGINDECLARE` and `DATETIMESELECT`. — Zev Spitz, Oct 21 '15 at 12:58
You have `on error GoTo CloseConnection`, so no error will be reported. Only use error handling if you are going to actually do something with remove the line and discover the real error. Also learn to use the debugger and step through your code. — Nick.Mc, Oct 21 '15 at 13:00
Try printing your SQL string - you're missing spaces between words! — Dave.Gugg, Oct 21 '15 at 13:22
And why are you using a while loop for this? There is no need for a while loop to insert data. You should use a set based approach instead. — Sean Lange, Oct 21 '15 at 13:35
An unrelated suggestion: Since VBA doesn't support multiline strings, use the line-continuation character to separate the concatenation into multiple lines. For an example, see [here](http://stackoverflow.com/a/17676050/111794). Alternatively, you could use the [Array](https://msdn.microsoft.com/en-us/library/office/gg264844.aspx) and [Join](https://msdn.microsoft.com/en-us/library/office/gg264098.aspx) functions together with the `_`: `sql = Join(Array("SELECT *", _ "FROM tbl1"), vbcrlf)` and you won't have to worry about spaces between the lines. — Zev Spitz, Oct 21 '15 at 13:38

score 0 · Accepted Answer · answered Oct 21 '15 at 14:01

You made two mistakes in SQL string construction:

you forgot about the newlines
you intermix SQL and variables

The first one is easy to fix. I prefer the following approach to create multi-line strings in VBA:

str = Join(Array( _
    "line", _
    "line", _
    "line" _
), vbNewLine)

The second mistake is the main reason for SQL injection vulnerabilities and as a matter of principle you should absolutely never use string concatenation to build variable SQL.

The ADODB.Command object exists for this. It takes a fixed string with ? placeholders and separate query parameters. It will combine query and parameters in a such a way that you won't ever need to worry about escaping and have type safety at the same time.

Option Explicit

Dim conn As New ADODB.Connection

Sub ConnectSqlServer()
    Dim Server_Name As String
    Dim Database_Name As String
    Dim User_ID As String
    Dim Password As String

    Server_Name = "******" ' Enter your server name here
    Database_Name = "******" ' Enter your database name here
    User_ID = "*****" ' enter your user ID here
    Password = "*****" ' Enter your password here

    conn.ConnectionString = "Provider=SQLNCLI10;Server=" & Server_Name & ";Database=" & Database_Name & ";Uid=" & User_ID & ";Pwd=" & Password & ";"
    conn.Open
End Sub

Sub FillSheet()
    Dim year As Integer

    If conn.State <> adStateOpen Then ConnectSqlServer()

    year = Application.InputBox("Enter the Year of choice ?", Type:=1)

    If year > 0 Then
        Sheets("Sheet3").Range("D4:O4").CopyFromRecordset GetYear(year)
    End If
End Sub

Function GetYear(year As Integer) As ADODB.Recordset
    ' TODO add a check for "conn Is Nothing"
    ' TODO add a check for "conn.State <> adStateOpen"
    With New ADODB.Command
        Set .ActiveConnection = conn
        .ActiveConnection.CursorLocation = adUseClient

        .CommandType = adCmdText
        .CommandText = Join(Array( _
            "DECLARE @Test TABLE (", _
                "ID INT IDENTITY(1, 1),", _
                "Value Float", _
            ")", _
            "DECLARE @year INT", _
            "DECLARE @InputDate DATETIME", _
            "DECLARE @MonthStartDate DATETIME", _
            "SET year = ?", _
            "SET @InputDate = CAST(@year + '-01-01' AS DATETIME)", _
            "WHILE @InputDate <= CAST(@year + '-12-01' AS DATETIME)", _
            "BEGIN", _
                "SELECT @MonthStartDate = CAST(DATEADD(dd, - DATEPART(dd, @InputDate) + 1, @InputDate)AS DATETIME)", _
                "INSERT INTO @Test", _
                "SELECT MAX([Value])*2 FROM DataLog2", _
                "WHERE DateAdd(HOUR,2,TimestampUTC) >= @MonthStartDate AND DateAdd(HOUR,2,TimestampUTC) < DATEADD(DAY,1,@MonthStartDate) AND SourceID = 26 AND quantityid = 129", _
                "SET @InputDate = DATEADD(MONTH, 1, @InputDate)", _
            "END", _
            "SELECT Value FROM @Test" _
        ), vbNewLine)

        .Parameters.Append .CreateParameter(Type:=adInteger, Value:=year)

        Set GetYear = .Execute ' .Execute() will always return an adLockReadOnly|adOpenForwardOnly RS
    End With
End Function

P.S.: @SeanLange's comment is correct, your SQL should not contain a while loop for the INSERT. You should modify it into a set-based statement.

In general you are correct about SQL injection, but in this specific case there is no danger because the `Year` variable is typed as an integer. You might want to refine this to _concatenating SQL and user string input_. — Zev Spitz, Oct 21 '15 at 14:36
@Zev I know, but code tends to get copied and used as a basis for custom extension. Code that sits on StackOverflow doubly so. I said "as a matter of principle" not without reason. Not ever building SQL from concatenating variable input of any kind, user-supplied or not, no exceptions, is a useful habit to get into. — Tomalak, Oct 21 '15 at 14:55
@Tomalak Thank you for the help and thanks to the rest of the guys. Reason for me using the while loop because it the only way I can get the month incremented for me to get the first weekday of each month. And the other problem that I kept getting if the query ran was because of the counts in the SQL so added SET NOCOUNT ON and made the whole query a sp. — WvdW, Oct 22 '15 at 11:38
What's wrong with using a date range? Why would you loop over every date between X and Y when you simply can insert all records between X and Y in one step? — Tomalak, Oct 22 '15 at 12:25

VBA Query returning no values

1 Answers1