1
\$\begingroup\$

I am using the following VBA to fetch data from an Access database. The query works but there is a lot of information in the database and the response time is very slow, about 30 seconds.

Is there any part of the code that I could optimise or write differently?

Sub test()

Dim strsql1, tr As String
Dim rst As ADODB.Recordset

strsql1 = "SELECT * FROM FP WHERE SUBMISSION_ID = 5683;"


Set cn = New ADODB.Connection
cn.Open "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Set rst = New ADODB.Recordset

rst.Open strsql1, cn, adOpenStatic

End Sub
\$\endgroup\$
2
  • 3
    \$\begingroup\$ There really isn't anything to optimize in the posted code in terms of performance. If opening the connection and fetching the result takes 30 seconds, then it takes 30 seconds. If you told us about the FP table, what indexes it has, what you intend to do with the result, then maybe we could help. \$\endgroup\$ Commented Jul 28, 2016 at 9:36
  • \$\begingroup\$ The only other suggestion I would have is to optimise the database, and only return the necessary fields, rather than *. \$\endgroup\$
    – SeanR
    Commented Jul 28, 2016 at 14:57

2 Answers 2

6
\$\begingroup\$

There's nothing to optimise here.

Your steps are:

Open connection  

Execute about the simplest SQL query possible  

End  

There's nothing we can change there that would make any difference.

Now, if you want to post a separate question, detailing your Database Structure, your SQL query, and asking how to optimise it, we could help there.


That aside, thoughts on your code:


You never use tr.


This:

Dim strsql1, tr As String

Does not actually Dim both as strings. When you declare variables, each variable must have a datatype or it will default to Variant.

This is what your code is actually doing, implicitly:

Dim strsql1 [As Variant], tr As String

Your database connection string should be a constant

If you're targeting a database, and you don't need to build the connection string dynamically, you should just have a project constant like so:

Public const FP_DATABASE_CONNECTION_STRING As String = "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Now it's in one place, and you know where it is, and if it ever changes, you know where to go to change it.


You should name your query

strsql1 tells me absolutely nothing useful.

selectFpId5683 would be a much better name. Sure, it's a bit verbose, but it actually lets us know what the variable is. And now, when we use it later, we can know that we're using the right thing.

selectFpId5683 = "SELECT * FROM FP WHERE SUBMISSION_ID = 5683;"

And now:

Option Explicit

Public Const FP_DATABASE_CONNECTION_STRING As String = "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Function GetAllFpRecordsWithId5683() As ADODB.Recordset

    Dim fpConnection As ADODB.Connection
    Set fpConnection = New ADODB.Connection
    fpConnection.Open FP_DATABASE_CONNECTION_STRING

    Dim selectFpId5683 As String
    selectFpId5683 = "SELECT * FROM FP WHERE SUBMISSION_ID = 5683;"

    Dim recordsWhereId5683 As ADODB.Recordset
    Set recordsWhereId5683 = New ADODB.Recordset
    recordsWhereId5683.Open selectFpId5683, fpConnection, adOpenStatic

    Set GetAllFpRecordsWithId5683 = recordsWhereId5683

End Function

Much, much easier to understand, work with, debug etc.


And then make it even better:

You'll probably want to target more than 1 ID in the future, so why not make your function a general one that can target any ID?

Option Explicit

Public Const FP_DATABASE_CONNECTION_STRING As String = "Provider=Microsoft.ace.OLEDB.12.0; Data Source=\\REDACTED.accdb;"

Function GetAllFpRecordsWithId(ByVal targetID As String) As ADODB.Recordset

    Dim fpConnection As ADODB.Connection
    Set fpConnection = New ADODB.Connection
    fpConnection.Open FP_DATABASE_CONNECTION_STRING

    Dim selectFpId As String
    selectFpId = "SELECT * FROM FP WHERE SUBMISSION_ID = " & targetID & ";"

    Dim recordsWhereId As ADODB.Recordset
    Set recordsWhereId = New ADODB.Recordset
    recordsWhereId.Open selectFpId, fpConnection, adOpenStatic

    Set GetAllFpRecordsWithId = recordsWhereId

End Function
\$\endgroup\$
1
  • \$\begingroup\$ Wouldnt DAO be a bit faster? I know this depends on the type of data set and volume etc etc but it seems for he wants there might be a slight performance \$\endgroup\$
    – Doug Coats
    Commented Sep 11, 2016 at 2:09
2
\$\begingroup\$

Nothing wrong with the code, I suppose you could record a macro in Excel to confirm but the answer, with regard to the time taken to retrieve data from the Access database can be down to the configuration and location of the Access database and the network infrasture between that and your Excel document.

Having good SQL / Code isn't going to change much if the database is located on an over used, under resourced server located at the end of a slow network connection.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.