iamjonesy
iamjonesy

Reputation: 25122

Inserting variables into a query string - it won't work!

Basically i have a query string that when i hardcode in the catalogue value its fine. when I try adding it via a variable it just doesn't pick it up.

This works:

  Dim WaspConnection As New SqlConnection("Data Source=JURA;Initial Catalog=WaspTrackAsset_NROI;User id=" & ConfigurationManager.AppSettings("WASPDBUserName") & ";Password='" & ConfigurationManager.AppSettings("WASPDBPassword").ToString & "';")

This doesn't:

Public Sub GetWASPAcr()

    connection.Open()

    Dim dt As New DataTable()
    Dim username As String = HttpContext.Current.User.Identity.Name
    Dim sqlCmd As New SqlCommand("SELECT WASPDatabase FROM dbo.aspnet_Users WHERE UserName = '" & username & "'", connection)

    Dim sqlDa As New SqlDataAdapter(sqlCmd)

    sqlDa.Fill(dt)

    If dt.Rows.Count > 0 Then

        For i As Integer = 0 To dt.Rows.Count - 1
            If dt.Rows(i)("WASPDatabase") Is DBNull.Value Then
                WASP = ""
            Else
                WASP = "WaspTrackAsset_" + dt.Rows(i)("WASPDatabase")
            End If

        Next

    End If
    connection.Close()

End Sub

Dim WaspConnection As New SqlConnection("Data Source=JURA;Initial Catalog=" & WASP & ";User id=" & ConfigurationManager.AppSettings("WASPDBUserName") & ";Password='" & ConfigurationManager.AppSettings("WASPDBPassword").ToString & "';")

When I debug the catalog is empty in the query string but the WASP variable holds the value "WaspTrackAsset_NROI"

Any idea's why?

Cheers,

jonesy

alt text http://www.freeimagehosting.net/uploads/ba8edc26a1.png

Upvotes: 1

Views: 469

Answers (6)

Thomas
Thomas

Reputation: 64645

I can see a few problems.

  1. You are using concatenation in a SQL statement. This is a bad practice. Use a parameterized query instead.
  2. You are surrounding the password with single quotes. They are not needed and in fact, I'm surprised it even works assuming the password itself does not have single quotes.
  3. You should surround classes that implement IDisposable with a Using block
  4. You should recreate the WASP connection object in GetWASPcr like so:
Public Sub GetWASPAcr()
    Dim username As String = HttpContext.Current.User.Identity.Name
    Dim listOfDatabaseConnectionString As String = "..."

    Using listOfDatabaseConnection As SqlConnection( listOfDatabaseConnectionString )
        Using cmd As New SqlCommand("SELECT WASPDatabase FROM dbo.aspnet_Users WHERE UserName = @Username")
            cmd.Parameters.AddWithValue( "@Username", username )

            Dim dt As New DataTable()
            Using da As New SqlDataAdapter( cmd )
                da.Fill( dt )

                If dt.Rows.Count = 0 Then
                    WaspConnection = Null
                Else
                    Dim connString As String = String.Format("Data Source=JURA;Initial Catalog={0};User Id={1};Password={2};" _ 
                        , dt.Rows(0)("WASPDatabase") _ 
                        , ConfigurationManager.AppSettings("WASPDBUserName") _ 
                        , ConfigurationManager.AppSettings("WASPDBPassword"))

                    WaspConnection = New SqlConnection(connString);
                End If  
            End Using
        End Using
    End Using
End Sub

In this example, listOfDatabaseConnectionString is the initial connection string to the central database where it can find the catalog name that should be used for subsequent connections.

All that said, why would you need a class level variable to hold a connection? You should make all your database calls open a connection, do a sql statement, close the connection. So, five database calls would open and close a connection five times. This sounds expensive except that .NET gives you connection pooling so when you finish with a connection and another is requested to be opened, it will pull it from the pool.

Upvotes: 4

Jan Willem B
Jan Willem B

Reputation: 3806

The problem is, as Paddy already points out, that the WaspConnection object gets initialized before you even have the chance to call GetWASPAcr. Try this:

Public Sub GetWASPAcr()
   '[...]

End Sub

Dim _waspConnection As SqlConnection
Public Readonly Property WaspConnection As SqlConnection
  Get
    If _waspConnection Is Nothing Then
      GetWASPAcr()
      _waspConnection = New SqlConnection("Data Source=JURA;Initial Catalog=" & WASP & ";User id=" & ConfigurationManager.AppSettings("WASPDBUserName") & ";Password='" & ConfigurationManager.AppSettings("WASPDBPassword").ToString & "';")
    End If
    Return _waspConnection
  End Get
End Property

Upvotes: 0

Steve McLain
Steve McLain

Reputation: 94

At the time you're creating the new connection, WASP is holding the value you want it to be holding? It is a string data type? Try adding .ToString after WASP and see if that helps anything.

Interesting problem. =-)

Upvotes: 0

DRapp
DRapp

Reputation: 48139

[link text][1]You are building your string on the fly by adding the value of a column to a string. So, for the row in question for the column "WASPDatabase" was tacked on to your string. So you got what it had. On another note, your earlier query of "select ... from ... where ..." where you are manually concatinating the string of a variable makes you WIDE OPEN to SQL-Injection attacks.

Although this link [1]: how to update a table using oledb parameters? "Sample query using parameterization" is to a C# sample of querying with parameterized values, the similar principles apply to most all SQL databases.

Upvotes: 0

edosoft
edosoft

Reputation: 17271

Might want to quit looking one you have found your database:

For i As Integer = 0 To dt.Rows.Count - 1
            If dt.Rows(i)("WASPDatabase") Is DBNull.Value Then
                WASP = ""
            Else
                WASP = "WaspTrackAsset_" + dt.Rows(i)("WASPDatabase")
                break
            End If

        Next

Upvotes: 0

Paddy
Paddy

Reputation: 33857

Your string passed into the constructor for this SqlConnection object will be evaluated when the class is instantiated. Your WASP variable (I'm assuming) won't be set until the method you have shown is called.

Upvotes: 1

Related Questions