On Sun, Jul 7, 2013 at 10:35 AM, Robins Tharakan <thara...@gmail.com> wrote: > On 26 June 2013 01:55, Robins Tharakan <thara...@gmail.com> wrote: >> >> Code coverage improved from 36% to 68%. > > Updated patch: > - Renamed ROLEs as per Robert's feedback (prepend regress_xxx) > - Added test to serial_schedule (missed out earlier).
Databases - like roles - are global objects, so those should be similarly prefixed with "regress". This is very dangerous: +DROP DATABASE db_db2; -- doesn't exist +DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS; +DROP DATABASE template1; -- can't drop a template database +DROP DATABASE regression; -- can't drop a database in use I think we should drop the first three of these. The first one risks dropping actual user data in the "make installcheck" case, as does the second one. We can reduce the risk of death and destruction by choosing a better database name, but I don't think it's really worth it. If DROP DATABASE gets broken, we'll notice that soon enough. I don't think the third one is a good test, either. There's no requirement that the user keep template1 around, although nearly everyone probably does. Please see if you can't also do something to minimize the number of create/drop role cycles this patch performs - like maybe to just one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
