Classic ASP - When to close recordset

Question

I would like to know, which of the following examples is the best for closing a recordset object in my situation?

1)

This one closes the object inside the loop but opens a new object when it moves next. If there were 1000 records, this opens an object 1000 times and closes it 1000 times. This is what I would normally do:

SQL = " ... "
Set rs1 = conn.Execute(SQL)
While NOT rs1.EOF

    SQL = " ... "
    Set rs2 = conn.Execute(SQL)
    If NOT rs2.EOF Then
        Response.Write ( ... )
    End If
    rs2.Close : set rs2 = Nothing

rs1.MoveNext
Wend
rs1.Close : Set rs1 = Nothing

2)

This example is what I want to know about. Does saving the object closure (rs2.close) until after the loop has finished, gains or reduces performance? If there were 1000 records, this would open 1000 objects but only closes it once:

SQL = " ... "
Set rs1 = conn.Execute(SQL)
While NOT rs1.EOF

    SQL = " ... "
    Set rs2 = conn.Execute(SQL)
    If NOT rs2.EOF Then
        Response.Write ( ... )
    End If

rs1.MoveNext
Wend
rs1.Close : Set rs1 = Nothing
rs2.Close : set rs2 = Nothing

I hope I've explained myself well enough and it's not too stupid.

UPDATE

To those who think my query can be modified to avoid the N+1 issues (2nd query), here it is:

This is for an online photo library. I have two tables; "photoSearch" and "photos". The first, "photoSearch", has just a few columns and contains all searchable data for the photos, such as "photoID", "headline", "caption", "people", "dateCaptured" and "keywords". It has a multi-column full-text index on (headline, caption, people, keywords). The second table, "photos", contains all of the photos data; heights, widths, copyrights, caption, ID's, dates and much more. Both have 500K+ rows and the headline and caption fields sometimes return 2000+ characters.

This is approximately how the query looks now: (things to note: I cannot use joins with fulltext searching, hence keywords being stored in one column - in a 'de-normalized' table. Also, this kind of pseudo code as my app code is elsewhere - but it's close )

SQL = "SELECT photoID FROM photoSearch
WHERE MATCH (headline, caption, people, keywords)
AGAINST ('"&booleanSearchStr&"' IN BOOLEAN MODE)
AND dateCaptured BETWEEN '"&fromDate&"' AND '"&toDate&"' LIMIT 0,50;"
Set rs1 = conn.Execute(SQL)
While NOT rs1.EOF

    SQL = "SELECT photoID, setID, eventID, locationID, headline, caption, instructions, dateCaptured, dateUploaded, status, uploaderID, thumbH, thumbW, previewH, previewW, + more FROM photos LEFT JOIN events AS e USING (eventID) LEFT JOIN location AS l USING (locationID) WHERE photoID = "&rs1.Fields("photoID")&";"
    Set rs2 = conn.Execute(SQL)
    If NOT rs2.EOF Then
        Response.Write ( .. photo data .. )
    End If
    rs2.Close

rs1.MoveNext
Wend
rs1.Close

When tested, having the full-text index on its own table, "photoSearch", instead of the large table, "photos", seemed to improve speed somewhat. I didn't add the "photoSearch" table, it was already there - this is not my app. If I try joining the two tables to lose the second query, I lose my indexing all together, resulting in very long times - so I can't use joins with full-text. This just seemed to be the quickest method. If it wasn't for the full-text and joining problems, I would have combined both of these queries already.

Rizo

Answer 1

Here is the thing. First, get your photo ids and make mysql thinks that is an actual table that hold the photo ids only, and then make your actual statement, no need any extra recordset connections...

And do not forget to start from the end to do this. Here is the sample code with explanations:

Step 1 Create photo ids lookup table and name it: This will our PhotoId Lookup Table so name it as "PhotoIds"

SELECT photoID FROM photoSearch
WHERE MATCH (headline, caption, people, keywords)
AGAINST ('"&booleanSearchStr&"' IN BOOLEAN MODE)
AND dateCaptured BETWEEN '"&fromDate&"' AND '"&toDate&"' LIMIT 0,50) AS PhotoIds

Step 2 Now we have photo ids, so get the informations from it. We will insert the above statement just before WHERE clause the same way as we do with real tables. Note that our "fake" table must be between parantheses.

SQL = "SELECT p.photoID, p.setID, p.eventID, p.locationID, p.headline, p.caption, + more FROM
    photos AS p,
    events AS e USING (p.eventID),
    location AS l USING (p.locationID),
    (SELECT photoID FROM photoSearch WHERE MATCH (headline, caption, people, keywords)
        AGAINST ('"&booleanSearchStr&"' IN BOOLEAN MODE) AND dateCaptured BETWEEN
        '"&fromDate&"' AND '"&toDate&"' LIMIT 0,50) AS PhotoIds
    WHERE p.photoID=PhotoIds.photoID;"

Note: I just write these codes here and never tested. There may be some spelling errors or smt. Please let me know if you have troubles.

Now getting your primary question

No need to close the executed queries, especially if you are using execute method. Execute method closes itself after the execution unless its not returning any recordset data (thats the purpose of execute command at the first place) like: "INSERT", "DELETE", "UPDATE". If you didnt open a recordset object, so why try to close something never opened? Instead you can use Set Rs=Nothing to unreference the object and send to the garbage collection to free up some system resources (and thats nothing to do with mysql itself). If you are using "SELECT" queries, (the queries that will return some data) you must Open a recordset object (ADODB.Recordset) and if you opened it, you need to close it as soon as it finishes its job.

The most important thing is to close the "main connection to mysql server" after each page load. So you may consider to put your connection close algorithm (not recordset close) to an include file and insert it at the end of everypage you make the connection to the database. The long talk short: You must use Close() if you used Open()

Cuboid

WOW! That new query makes sense and it works. I'm testing in MySQL Workbench and it's very quick. I need to add it to my app to see if it speeds things up. I have one question; where would I put an INNER JOIN in this query? Like this one; LEFT JOIN ( photoPeople AS pp INNER JOIN people AS pe ON pp.peopleID = pe.PeopleID) ON p.photoID = pp.photoID? – Rizo

@PaparazzoKid You don't have to use INEER JOIN, LEFT JOIN etc. when working with MySQL. You can join multiple tables with using "," (comma operator) and pseudo-name the tables. i.e: ("SELECT t1.name, t2.name FROM very_long_table_name_table1 AS t1, very_long_table_name_table2 AS t2 WHERE t1.id=t2.id") – Cuboid

Oh, that threw a spanner in the works. I have always used LEFT, RIGHT, JOIN or INNER JOINS for MySQL - I'll need to read about that. The reason I used LEFT was the tables I want to join won't always have a matching record. Would this be the same in your method? – Rizo

Actually, cancel that, I think I have just worked it out. It's all about the "WHERE t1.id = t2.id" - which acts as a left join I suppose. Thanks a lot for your help, you cleared a couple of things up. – Rizo

@PaparazzoKid The reason I used LEFT was the tables I want to join won't always have a matching record. If this is the case, you must use LEFT JOIN yes. But maybe something like ("SELECT t1.name, t2.name FROM very_long_table_name_table1 AS t1, very_long_table_name_table2 AS t2 WHERE t1.id=t2.id AND t2.name!=NULL") will also work instead of LEFT JOIN. If you need help about this subject, I suggest you to check mysql manual SELECT syntax reference and look for "comma operator". – Cuboid

Answer 2

If you show us your SQL Statements, maybe we can show you how to combine them into a single SQL statement so you only have to do one loop, otherwise, double looping like this really takes a toll on the servers performance. But before I learned Stored Procedures and Joins, I would have probably done it like this:

 Set Conn = Server.CreateObject("Adodb.Connection")
 Conn.Open "ConnectionString"

 Set oRS = Server.CreateObject("Adodb.Recordset")
 oRS.Open "SQL STATEMENT", Conn

 Set oRS2 = Server.CreateObject("Adodb.Recordset")
 oRS2.ActiveConnection = Conn

 Do Until oRS.EOF

    oRS2.Open "SQL STATEMENT"
    If oRS2.EOF Then ...
    oRS2.Close

 oRS.Movenext
 Loop
 oRS.Close
 Set oRS = Nothing
 Set oRS2 = Nothing
 Set Conn = Nothing

Wheatley

Agree 100% that the design of the query(ies) can likely be improved to avoid the inner query entirely. This is known as the 'select N+1 problem' #97697. You'll get better results if you focus on that issue rather than optimizing how the recordset objects are disposed of. – Rothmuller

I know, I remember these days.. Once I started combining SQL queries, the performance of the server significantly improved, by like 100x. – Wheatley

I'm actually getting far better results using my method. Tried and tested. Usually I would try and make two queries into one, just as you say, but it doesn't seem to work in my situation. My table has 500k rows and many, many columns, some quite large columns too (2000+ characters). When my page was using just the one query, it was selecting nearly all of the columns in that table and then displaying 50 records per page. The query took forever! I then changed to my above method, selecting only the ID from the top query and getting the heavy data from the second query inside the loop. – Rizo

@ZeeTee: I'm not double-looping in my example. It's only one loop but multiple recordsets. – Rizo

I'm also using full-text search for my top query. This disallows me to use any joins, as the index doesn't work. – Rizo

@PaparazzoKid By the looks of your question, I'm almost positive it could be made more efficiently with one SQL Statement, rather than a double connection (not loop, you're right), but making this second connection, still consumes more resources than doing it on SQL. You should post your SQL statements and see if we can help you optimize it. – Wheatley

@ZeeTee: Question updated with pseudo-ish query. Thanks for your time. – Rizo

Answer 3

I tried putting this in a comment because it doesn't directly answer your original question, but it got too long.. :)

You could try using a sub-query instead of a join, nesting the outer query inside the second one. " ... where photoID in(select photoID from photoSearch ... )". Not sure if it would get better results, but it may be worth trying. That being said, the use of the full-text search does change how the queries would be optimized, so it may take more work to figure out what the appropriate indexes are (need to be). Depending on your existing performance, it may not be worth the effort.

Do you know for sure that this existing code/query is the current bottleneck? Sometimes we spend time optimizing things that we think are the bottleneck when that may not be the case... :)

One additional thought - you may want to consider some caching logic to reduce the amount of redundant queries you may be making - either at the page level or at the level of this method. The search parameters could be concatenated together to form the key for storing the data in a cache of some sort. Of course you would need to handle appropriate cache invalidation/expiry logic. I've seen systems speed up 100x with very simple caching logic added to bottlenecks like this.

Rothmuller

Caching sounds fun, may look into that further. I don't actually have any bottle necks on this particular page or query. I purely wanted to know what the best way to close a recordset object was, when used in a loop. After the loop or in the loop. – Rizo

ok - you did ask which was the best approach for performance, so I think it's natural for people to focus on the performance aspect of your code, and in this case the query has a much bigger impact than the handling of the recordset objects. As a general principle if you don't have a performance issue you probably shouldn't spend time optimizing it... :) – Rothmuller

Answer 4

It's simple ask about the state of your RecordSet is 1 or 0 , It means open or close

like this

If RS.State = 1 Then RS.Close
 

the connection to database (CN) will still up but you can reopen the RS (RecordSet) again with any values

Yoshida