ADODB Best Update Method?

dbay

Registered User.
Local time
Today, 15:21
Joined
Jul 15, 2007
Messages
87
I am in the process of moving my systems from Access backends to SQL Server. I am now learning about ADO methods and am getting overwhelmed with all the different methods to do the same thing. My question is which one is the more "Correct" method and why??

Code:
Private Sub Save_System()

    Dim ADO_CON As Object
    Dim ADO_COM As Object
    Dim STR_SQL As String

On Error GoTo err_proc
    
    Set ADO_CON = CreateObject("ADODB.Connection")
    With ADO_CON
        .ConnectionString = CON_SQL_Connection
        .Open
    End With
    
    If ADO_CON.state = adStateOpen Then

                  STR_SQL = "UPDATE "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS] "
        STR_SQL = STR_SQL & "SET "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Name] = '" & Me.TXT_System_Name.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Owner] = '" & Me.TXT_System_Owner.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Version] = '" & Me.TXT_System_Version.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Status] = '" & Me.CBO_System_Status.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_License] = '" & Me.TXT_System_License.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Path] = '" & Me.TXT_System_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Documents_Path] = '" & Me.TXT_Documents_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Employee_Image_Path] = '" & Me.TXT_Employee_Image_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Error_Path] = '" & Me.TXT_Error_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Frontend_Path] = '" & Me.TXT_Frontend_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Group_Image_Path] = '" & Me.TXT_Group_Image_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Icons_Path] = '" & Me.TXT_Icons_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Install_Path] = '" & Me.TXT_Install_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Templates_Path] = '" & Me.TXT_Templates_Path.Value & "', "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Excel_Password] = '" & Me.TXT_Excel_Password.Value & "' "
        STR_SQL = STR_SQL & "WHERE "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Index] = " & Me.TXT_System_Index.Value & ""
        
        Set ADO_COM = CreateObject("ADODB.Command")
        With ADO_COM
            .ActiveConnection = ADO_CON
            .CommandType = adCmdText
            .CommandText = STR_SQL
            .Prepared = False
            .Execute , , adExecuteNoRecords
        End With
        
    Else
        MsgBox ("Server Timed Out!"), vbCritical, "SERVER ERROR"
    End If

exit_proc:
On Error Resume Next
    ADO_CON.Close
    Set ADO_CON = Nothing
    Set ADO_COM = Nothing
    Exit Sub

err_proc:
    Error_Logging STR_Form:=Me.Name, STR_Procedure:="Save_System", LNG_Line:=Erl, LNG_Number:=Err.Number, STR_Description:=Err.Description
    Resume exit_proc

End Sub
Code:
Private Sub Save_System()

    Dim ADO_CON As Object
    Dim ADO_COM As Object
    Dim STR_SQL As String

On Error GoTo err_proc

    Set ADO_CON = CreateObject("ADODB.Connection")
    With ADO_CON
        .ConnectionString = CON_SQL_Connection
        .Open
    End With

    If ADO_CON.state = adStateOpen Then

                  STR_SQL = "UPDATE "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS] "
        STR_SQL = STR_SQL & "SET "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Name] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Owner] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Version] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Status] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_License] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Documents_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Employee_Image_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Error_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Frontend_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Group_Image_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Icons_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Install_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Templates_Path] = ?, "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_Excel_Password] = ? "
        STR_SQL = STR_SQL & "WHERE "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Index] = ?"

        Set ADO_COM = CreateObject("ADODB.Command")
        With ADO_COM

            .ActiveConnection = ADO_CON
            .CommandType = adCmdText
            .CommandText = STR_SQL
            .Prepared = False

            .Parameters.Append .CreateParameter("SYS_System_Name", adVarChar, adParamInput, 50, Me.TXT_System_Name.Value)
            .Parameters.Append .CreateParameter("SYS_System_Owner", adVarChar, adParamInput, 50, Me.TXT_System_Owner.Value)
            .Parameters.Append .CreateParameter("SYS_System_Version", adVarChar, adParamInput, 25, Me.TXT_System_Version.Value)
            .Parameters.Append .CreateParameter("SYS_System_Status", adVarChar, adParamInput, 25, Me.CBO_System_Status.Value)
            .Parameters.Append .CreateParameter("SYS_System_License", adVarChar, adParamInput, 25, Me.TXT_System_License.Value)
            .Parameters.Append .CreateParameter("SYS_System_Path", adVarChar, adParamInput, 255, Me.TXT_System_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Documents_Path", adVarChar, adParamInput, 255, Me.TXT_Documents_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Employee_Image_Path", adVarChar, adParamInput, 255, Me.TXT_Employee_Image_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Error_Path", adVarChar, adParamInput, 255, Me.TXT_Error_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Frontend_Path", adVarChar, adParamInput, 255, Me.TXT_Frontend_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Group_Image_Path", adVarChar, adParamInput, 255, Me.TXT_Group_Image_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Icons_Path", adVarChar, adParamInput, 255, Me.TXT_Icons_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Install_Path", adVarChar, adParamInput, 255, Me.TXT_Install_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Templates_Path", adVarChar, adParamInput, 255, Me.TXT_Templates_Path.Value)
            .Parameters.Append .CreateParameter("SYS_Excel_Password", adVarChar, adParamInput, 255, Me.TXT_Excel_Password.Value)
            .Parameters.Append .CreateParameter("SYS_System_Index", adInteger, adParamInput, , Me.TXT_System_Index.Value)
            .Execute , , adExecuteNoRecords

        End With

    Else
        MsgBox ("Server Timed Out!"), vbCritical, "SERVER ERROR"
    End If

exit_proc:
On Error Resume Next
    ADO_CON.Close
    Set ADO_CON = Nothing
    Set ADO_COM = Nothing
    Exit Sub

err_proc:
    Error_Logging STR_Form:=Me.Name, STR_Procedure:="Save_System", LNG_Line:=Erl, LNG_Number:=Err.Number, STR_Description:=Err.Description
    Resume exit_proc

End Sub
Code:
Private Sub Save_System()

    Dim ADO_CON As Object
    Dim ADO_RS As Object
    Dim STR_SQL As String

On Error GoTo err_proc

    Set ADO_CON = CreateObject("ADODB.Connection")
    With ADO_CON
        .ConnectionString = CON_SQL_Connection
        .CursorLocation = adUseServer
        .CommandTimeout = 30
        .ConnectionTimeout = 15
        .Open
    End With

    If ADO_CON.state = adStateOpen Then

                  STR_SQL = "SELECT * "
        STR_SQL = STR_SQL & "FROM "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS] "
        STR_SQL = STR_SQL & "WHERE "
        STR_SQL = STR_SQL & "[SYSTEM_NAME].[dbo].[TBL_SYSTEMS].[SYS_System_Index] = " & Me.TXT_System_Index.Value & ""

        Set ADO_RS = CreateObject("ADODB.Recordset")
        With ADO_RS

            .Source = STR_SQL
            .ActiveConnection = ADO_CON
            .CursorType = adOpenKeyset
            .LockType = adLockOptimistic
            .Open , , , , adCmdText

            If Not (.BOF And Not .EOF) Then
                If .Supports(adUpdate) Then

                    ![SYS_System_Name] = Me.TXT_System_Name.Value
                    ![SYS_System_Owner] = Me.TXT_System_Owner.Value
                    ![SYS_System_Version] = Me.TXT_System_Version.Value
                    ![SYS_System_Status] = Me.CBO_System_Status.Value
                    ![SYS_System_License] = Me.TXT_System_License.Value
                    ![SYS_System_Path] = Me.TXT_System_Path.Value
                    ![SYS_Documents_Path] = Me.TXT_Documents_Path.Value
                    ![SYS_Employee_Image_Path] = Me.TXT_Employee_Image_Path.Value
                    ![SYS_Error_Path] = Me.TXT_Error_Path.Value
                    ![SYS_Frontend_Path] = Me.TXT_Frontend_Path.Value
                    ![SYS_Group_Image_Path] = Me.TXT_Group_Image_Path.Value
                    ![SYS_Icons_Path] = Me.TXT_Icons_Path.Value
                    ![SYS_Install_Path] = Me.TXT_Install_Path.Value
                    ![SYS_Templates_Path] = Me.TXT_Templates_Path.Value
                    ![SYS_Excel_Password] = Me.TXT_Excel_Password.Value
                    .Update

                Else
                    MsgBox ("Record Locked by Another User!"), vbCritical, "UPDATE ERROR "
                End If
            End If

        End With

    Else
        MsgBox ("Server Timed Out!"), vbCritical, "SERVER ERROR"
    End If

exit_proc:
On Error Resume Next
    ADO_CON.Close
    Set ADO_CON = Nothing
    ADO_RS.Close
    Set ADO_RS = Nothing
    Exit Sub

err_proc:
    Error_Logging STR_Form:=Me.Name, STR_Procedure:="Save_System", LNG_Line:=Erl, LNG_Number:=Err.Number, STR_Description:=Err.Description
    Resume exit_proc

End Sub
 
theres no need for any of this code.
you can do all this with queries.
 
Second method is definitely better. You do not want to mess with properly quoting text values in SQL. The second approach with ADODB.Command gives you a portable code that you can later use in ASP if required.
 
I think it would be better to use stored procedures, it would be very easy to do some severe damage to your database with sql injection using any of those methods.
 
I think it would be better to use stored procedures, it would be very easy to do some severe damage to your database with sql injection using any of those methods.

Are you suggesting all transactions should be done with stored procedures?
 
I think it would be better to use stored procedures, it would be very easy to do some severe damage to your database with sql injection using any of those methods.

Concatenating a command is certainly dangerous but I don't think an injection attack would succeed using a parameterised query. If it were possible, the Stored Procedure would be equally vulnerable.

But a Stored Procedure is far more efficient to execute because it already has its execution plan saved.

BTW I would advise that the OP learns more about making their code readable.

Firstly, there is no need to qualify the column name with the table since there is only one table in the query. (Alias the table names to something shorter in cases where the name needs to be qualified.)

Use line continuations instead of the ridiculous repeated concatenation. You can do up to 22 continuations and it is a lot more readable.

Use a With Block instead of repeating the same method over and over.
 
Actually you are right, I was thinking they could stick something on the end of the where clause parameter, but looking now I realise I made a mistake because that parameter is actually an integer anyway.

Using stored procedures is always good practice anyway, and I always validate my input parameters (where possible) in stored procedures.

However the bit about stored procedures being more efficient, isn't correct, they used to behave differently in terms of caching execution plans, but since SQL server 2005 ad hoc queries and stored procedures both work in the same way, the execution plan is always cached.

I agree with the making the code more readable and reusable.
 
Last edited:
However the bit about stored procedures being more efficient, isn't correct, they used to behave differently in terms of caching execution plans, but since SQL server 2005 ad hoc queries and stored procedures both work in the same way, the execution plan is always cached.

How can an ad hoc query have a cached execution plan?

At the very least it would require that the server determine that the current ad hoc query had been used before and match it to a potentially huge list of previously executed queries. This process would surely cause a delay compared to a named procedure.
 
No it wouldn't because a stored procedure has to go through exactly the same process, if the server has been rebooted or the plan cache has been wiped.

So if there's an existing plan it will be reused, and if there's no plan then a new one will be generated, this is applicable to both ad hoc queries and stored procedures.

"When any SQL statement is executed in SQL Server, the relational engine first looks through the procedure cache to verify that an existing execution plan for the same SQL statement exists. SQL Server reuses any existing plan it finds, saving the overhead of recompiling the SQL statement. If no existing execution plan exists, SQL Server generates a new execution plan for the query"

Taken from here

https://technet.microsoft.com/en-us/library/ms181055(v=sql.105).aspx
 

So the SQL itself is scanned. The execution plan doesn't belong to the query as such.

This bit really got my interest:

The algorithms to match new SQL statements to existing, unused execution plans in the cache require that all object references be fully qualified. For example, the first of these SELECT statements is not matched with an existing plan, and the second is matched:

Code:
SELECT * FROM Person;

Code:
SELECT * FROM Person.Person;
 
Last edited:
Good spot, that's news to me also. Very interesting indeed.

Better get updating my SQL Server standards documentation :)
 
Another question.

What happens when using a Command.Execute method to Update a record that has been locked or deleted by another user?

When using a Recordset you can check whether or not the record supports updating at the moment using the .Supports(adUpdate) property and if the record still exists by checking .BOF or EOF.

Is there a way to do similar checks when using the Command.Execute method?
 

Users who are viewing this thread

Back
Top Bottom