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
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 28, 2016 at 9:12
\$\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\$ 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
answered Jul 28, 2016 at 10:04
\$\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\$ 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.

answered Jul 28, 2016 at 12:25
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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.