On Fri, Sep 22, 2017 at 7:26 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > "Bossart, Nathan" <bossa...@amazon.com> writes: >> [ 0001-error_on_duplicate_columns_in_analyze_v6.patch ] > > I've pushed (and back-patched) the 0001 patch, with some significant > changes:
Thanks. Arrived here too late to give feedback on the proposed patch. > I'll take a look at the updated 0002 tomorrow, but if anyone living in > a different timezone wants to review it meanwhile, feel free. Here is some feedback. 0002 fails to apply after 0001 has been committed in its regression tests, that can easily be resolved. A couple of tests could be removed: +VACUUM (FREEZE, ANALYZE) vactst (i); +VACUUM (FREEZE, ANALYZE) vactst (i, i, i); +VACUUM vactst (i); void -vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params, - List *va_cols, BufferAccessStrategy bstrategy, bool isTopLevel) +vacuum(int options, VacuumRelation *relation, VacuumParams *params, + BufferAccessStrategy bstrategy, bool isTopLevel) [...] - relations = get_rel_oids(relid, relation); + relations = get_rel_oids(relation); I still think that ExecVacuum() should pass a list of VacuumRelation objects to vacuum(), and get_rel_oids() should take in input a list, and return a completed lists. This way the decision-making of doing everything in the same transaction should happens once in vacuum(). And actually, if several relations are defined with VACUUM, your patch would not use one transaction per table as use_own_xacts would be set to false. I think that Tom meant that relations whose processed has finished have to be committed immediately. Per your patch, the commit happens once all relations are committed. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
