Forum OpenACS Improvement Proposals (TIPs): TIP #31: (Approved for 5.1) Make db_multirow unclobber by default

I just spent 1-2 hours debugging a problem caused by db_multirow clobbering variables in its callers scope with the values in the last selected row. We have too much uncontrolled variable clobbering and too little variable namespace separation in general in OpenACS. The arguments in favor of this change have been exhaustively presented in this thread:

I believe any upgradability difficulties caused by this change (which I estimate to be small) in the long run will be outweighed by the improved robustness of the toolkit.

Posted by Tom Jackson on

Yes, I think that Lars pointed out that db_multirow, when used without the foreach loop doesn't clobber. I believe this was the original design, and then someone added the foreach loop, which was a great idea, but they added the clobbering features of db_foreach as well. This makes db_multirow act differently if you later decide to extend a row, and do so by using the foreach loop. Suddenly things start to break and you have to rethink your entire page.

The main purpose of db_multirow is to create the datasource in an array format, so the question really is whether the the switch should be clobber or unclobber?

If clobbering is the default, I doubt it will quicken bug squashing caused by forgetting the switch. However folks will quickly find the cause if unclobbering is the default, since none of their variable will exist if they thought they should.

One other point: if clobbering is the default, a multirow will not leave the page in a predictable state. it will depend on the last row of the select statement. The only reason to use a db_multirow in this case is when you really want to use db_1row and also use the features of db_foreach/db_multirow.

Posted by Jeff Davis on
I vote yes with noclobber as the default but not until 5.1 since I think there is too much testing to do to make sure it works.
Posted by Lars Pind on
Approved for 5.1.