0

I have this code which works:

sqlQuery = ("SELECT monthdata.VAL, monthdata.MONTHVAL, monthdata.GREEN, monthdata.RED, monthdata.RAG, monthdata.CREATOR FROM data LEFT JOIN monthdata ON data.UID = monthdata.DATAUID WHERE [UID] = '" & (IDcell) & "' AND [MONTHVAL] = #" & Format((month1), "mm/dd/yyyy") & "#")

I'm specifically interested in the end, i.e this bit:

[MONTHVAL] = #" & Format((month1), "mm/dd/yyyy") & "#")

I now have a variable "month1formatted" which is already in mm/dd/yyyy format. So I tried to put this in as a replacement and lose the formatting bit. This is what I ended up with:

sqlQuery = ("SELECT monthdata.VAL, monthdata.MONTHVAL, monthdata.GREEN, monthdata.RED, monthdata.RAG, monthdata.CREATOR FROM data LEFT JOIN monthdata ON data.UID = monthdata.DATAUID WHERE [UID] = '" & (IDcell) & "' AND [MONTHVAL] = month1formatted"

However, strangley this doesn't work. Am I putting the variable in at the end in the wrong way?

When I print the variable sqlQuery it is printing the text name of the variable (i.e AND [MONTHVAL] = month1formatted), not the value of the variable but I know the variable is set correctly

Jimmy
  • 12,087
  • 28
  • 102
  • 192
  • 1
    How did you declare the month1? it's tricky doing things with dates... i'd declare the variable holding it as string and see if it returns the right format – Damian Oct 04 '18 at 16:48
  • Like this ```currentdate = Worksheets("Data").Cells(6, "A").Value month1 = Format(currentdate, "mm/dd/yyyy")``` Edited my comment above – Jimmy Oct 04 '18 at 16:49
  • 2
    You should parameterize your query. Think `IDcell = "' GO; DROP TABLE data --"` – Comintern Oct 04 '18 at 16:52

1 Answers1

3

Nothing strange about it, you've embedded a variable into your SQL string that your SQL engine knows nothing about.

Replace Format((month1), "mm/dd/yyyy") in your original string with month1formatted from your new string and the magic will happen.

sqlQuery = "SELECT monthdata.VAL, monthdata.MONTHVAL, monthdata.GREEN, " & _
           "       monthdata.RED, monthdata.RAG, monthdata.CREATOR " & _
           "  FROM data " & _
           "       LEFT JOIN monthdata ON data.UID = monthdata.DATAUID " & _
           " WHERE [UID] = '" & IDcell & "' " & _
           "  AND [MONTHVAL] = #" & month1formatted & "#"

Also, no need for the enclosing parenthesis () around your string literal, and I formatted it with line breaks to ease readability.

Oh, and do as Comintern instructed and parameterize the query! This accepted answer is a good example of how to do that.

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
FreeMan
  • 5,660
  • 1
  • 27
  • 53
  • Thank you very much – Jimmy Oct 04 '18 at 17:27
  • 2
    The use of parameterized queries (as "instructed" by the "Comintern") is a mean of avoid (or reduce the occurrence of) SQL-injection... Is highly recomended to follow the recommendations of the "accepted answer" – Álvaro Gustavo López Oct 04 '18 at 17:39
  • @ÁlvaroGustavoLópez - thank you for clarifying that. I should have done so. – FreeMan Oct 04 '18 at 17:41
  • I've seen in industrial software -supposedly written by professionals in the field- to make this mistake (SQL injection)... Indeed, I have had a dispute with a supplier about this matter. The experience, after all -even in such a "modern" area- is a good teacher... it isn't? – Álvaro Gustavo López Oct 05 '18 at 11:53