Created attachment 3500 [details]
Old patch - use newer one

Comment on attachment 3500 [details]
Old patch - use newer one

New version of patch posted below.

Created attachment 3501 [details]
v1.1 of patch to enable persistent SQL handles and statements

Hmm.. turns out there are more changes needed to impliment this than I thought.
The database handles themselves are not Persistent, as the factory->finish()
routine is called after each use. In addition, the new_checker routine creates
a new DB handle each time.

I've created a newer patch that a) fixes those things in SQLBasedAddrList.pm
and b) passes the $main environment in AutoWhitelist.pm so that we can create
the database handle ONCE during the factory->new() call. Tested and working
properly for me.

http://wiki.apache.org/spamassassin/DBIPlugin

(In reply to comment #4)
> http://wiki.apache.org/spamassassin/DBIPlugin

I think these are complimentary means of dealing with the issue, not substitute 
(ie, both could be used).

Global DBI caching is indeed important, and this type of Ppugin will handle 
things like auto_reconnect and ping testing (auto_reconnect was something I 
didn't put in the patch above simply to make the patch cleaner and more 
understandable), but I also think that a "Mail::SpamAssassin::SQLBasedAddrList" 
file that subclasses off "Mail::SpamAssassin::PersistentAddrList" should be 
modified to not set up and tear down its statements and connections itself each 
time through -- especially since the DB configuration is <a 
href="http://search.cpan.org/~felicity/Mail-SpamAssassin-
3.1.1/lib/Mail/SpamAssassin/Plugin/AWL.pm#ADMINISTRATOR_SETTINGS">by 
definition, not changable on a per-user basis</a>.

Any update on this bug? Patch still applies cleanly to 3.1.4.

Although a long-term DBI caching solution is still important, there's no need 
for new logical statement handles to be explicitly created each time through 
this Module. The current code seems like it will need to be changed eventually, 
and there is only one change outside of SQLBasedAddrList, which (shouldn't) 
affect anything else using the AutoWhitelist functions.

given that DBIPlugin cannot be put into the main distro, due to its incompatible
license, can we put this patch into trunk instead?  Michael?

Sorry to keep pinging on this, but has there been any further consideration on 
this bug? The patch continues to apply cleanly to the most recent SA release 
(3.1.5) and has been in production at our site since this bug was opened.