2

i'm trying to get the Max field where date = today using this code:

    Dim todaydate = Format(Today.Date, "dd/MM/yyyy")
    Dim sql1 As String = "Select max(snum) From tblbill where idate =  #" & todaydate & "# "
    Dim conn1 As SqlConnection = New SqlConnection(constr)
    Dim cmd1 As SqlCommand = New SqlCommand(sql1, conn1)
    conn1.Open()
    Dim dr1 As SqlDataReader = cmd1.ExecuteReader
    dr1.Read()
    If IsDBNull(dr1(0)) Then
        TextBox6.Text = 1
    Else
        TextBox6.Text = dr1(0) + 1
    End If
    dr1.Close()
    cmd1.Dispose()
    conn1.Close()

but when run the app i got this error: Incorrect syntax near '#'. may anyone help please!

Improve this question
6
  • 1
    # is the date delimiter for Access SQL, use single quotes for T-SQL (and perhaps a locale agnostic date format like yyyymmdd). Commented Feb 22, 2016 at 12:15
  • 2
    @AlexK. - this is bad advice because it still leaves this open to sql injection vulnerabilities. The correct advice is to use a parameterised query which does not require a locale specific date format at all. Commented Feb 22, 2016 at 12:33
  • Describe the attack that causes Format(Today.Date, "dd/MM/yyyy") to return an injectable string? Commented Feb 22, 2016 at 12:34
  • 1
    There isn't one in this case, but that doesn't mean you should be promoting bad practise. The OP might get this to work, then use this as the basis for a method that passes a date in from a form and bingo! Commented Feb 22, 2016 at 12:36
  • 1
    @Matt Wilko is correct, just because 'poor code' works doesn't make it right... if you adopt bad habits for simple tasks then you will always write poor code every time because you wont acquire the ability to write good robust code when it does matter... Commented Feb 22, 2016 at 13:26

2 Answers 2

Reset to default
9

First and foremost USE PARAMETERISED QUERIES, concatenating strings is vulnerable to malformed SQL, malicious SQL Injection, and conversion errors, in addition it stops query plan re-use because a new plan is created for every different value passed. This already solves your issue, because you do not need to worry about what qualifiers to use for what datatype (as pointed out in a comment you need to use ' instead of # which is for MS Access), it also means you don't need to worry about whether the format is DD/MM/YYYY or MM/DD/YYYY, you are telling the SqlCommand to expect a date, so regional settings will not affect anything.

Secondly, it is a good idea to use Using blocks to let your IDisposable objects clean themselves up:

Dim sql1 As String = "Select max(snum) From tblbill where idate =  @Date "
Using conn1 As New SqlConnection(constr)
Using cmd1 As New SqlCommand(sql1, conn1)

    cmd1.Parameters.Add("@Date", SqlDbType.DateTime).Value = Today.Date
    conn1.Open()

    Using dr1 As SqlDataReader = cmd1.ExecuteReader
        If IsDBNull(dr1(0)) Then
            TextBox6.Text = 1
        Else
            TextBox6.Text = dr1(0) + 1
        End If
    End Using

End Using
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Finally a sensible answer!
Yeah, that's a model answer, simple and to the point. 6 up-votes in under an hour so the consensus is silently proven, definitely worth bookmarking for future reference.....
-3

You need to use a single quote instead of hash here, it would be better if you could use string.format as well (instead of manually concatenating)

Dim sql1 As String = String.Format("select max(snum) from tblbill where idate='{0}'",todaydate)
Improve this answer

1 Comment

This will quite probably fail in a number of different locales. Also it is open to SQL injection so is bad practise

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.