Forum OpenACS Development: Can someone explain why nested queries such as db_foreach are not recommended?

Request notifications

Hi

I noticed this comment in Database Pools section of the openacs-config.tcl and I must confess that I don't fully understand it. (see https://bitbucket.org/naviserver/naviserver/raw/91b5b3dea68657f0a238eda2ddf206247b31759b/openacs-config.tcl )

"nested queries (i.e. in a db_foreach, which is actually not recommended)"

1. Does this mean that db_foreach should never be used, or that other queries should not be nested inside a db_foreach?
2. What is the thinking behind this recommendation?
3. What should be done as an alternative that would still support transactions?
4. Does this recommendation include other multiple row queries e.g db_multirow?

A few years back, a colleague of mine wrote a version of db_foreach called db_foreach_no_handle that supports nesting (see https://openacs.org/forums/message-view?message_id=3686120 ). However, I must admit that I don't know if this is a good solution from a transaction supporting point of view.

Any advice greatly appreciated!
Brian

There are several aspects:
a) transaction semantics
b) handle management
c) performance aspects

For (a): when there is a transaction with some outer query (using e.g. "db_exec select" and an inner query (also a transaction), how to control, the scope of the rollback?

b) If there are nested db_foreach, implemented via "db_exec select", then on each nesting level a new handle is fetched from a new pool. So there is an obvious problem with nestings (do API calls use this or not?) and with the number of handles needed per pool (how much handles should be allocated per pool?).

c) If one is using SQL calls inside a db_foreach, and the db_foreach returns e.g. 1000 rows, then the inner part of the loop executes 1000 SQL queries. Executing a high number of even quick SQL queries is often a performance issue. It is nearly always better to perform a more complex SQL query at the first place.

Part of the problem is the usage of "db_exec select" in the classical "db_foreach" implementation of OpenACS [1]. If one avoids this (by using e.g. e.g. db_list_of_litsts, or db_list_of_ns_sets [2]) in the implementation of db_foreach, then the same handle can be reused. Therefore, one is not running into the handle limitation, and the transaction semantics are clear.

As you can see [2], oacs-5-10 already avoids some of these issues. In application programs, looking at db_foreach constructs can help to get rid of performance issues. One can certainly continue to use db_foreach....

Hope, the helps more than it confuses.
all the best
-g

[1] https://github.com/openacs/openacs-core/blob/oacs-5-9/packages/acs-tcl/tcl/00-database-procs.tcl#L1403

[2] https://github.com/openacs/openacs-core/blob/oacs-5-10/packages/acs-tcl/tcl/01-database-procs.tcl#L1633

Many thanks Gustaf, for that response. The issue is clearer to me now. I wasn't aware that db_foreach had changed in oacs-5-10, so that is something definitely worth a look at.

all the best
Brian

I agree completely with everything Gustaf said above. But I'd emphasize some of it even more strongly:

I don't recall ever needing to use db_foreach. There are probably a tiny number of cases where using it is actually a good idea, but offhand I don't remember those ever occurring for me. IMO, db_foreach is in the API mostly as a crutch and a convenience, but you probably should not use it. In cases where you think you want it, you probably should be using db_list_of_lists or something else similar instead.

Thanks Andrew, that's good info.

Out of curiosity, I just checked my local copy of OpenACS 5.9.1. and found 334 occurrences of db_foreach in 170 files, so it seems to be quite popular!

Brian

db_foreach is mostly convenient when one has a query returning some big number of columns. In such case, when using e.g. db_list_of_lists, one would have to "peel" the results with a cumbersome lassign.

Using db_list_of_ns_sets would be a nicer replacement in this case, as it maintains a notion of which columns have been extracted in the ns_set. In fact, latest db_foreach implementation is just a wrapper around this api.