On Wed, Feb 26, 2014 at 12:29 PM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2014-02-24 17:06:53 -0500, Robert Haas wrote: >> - heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin); >> + if (IsSystemRelation(scan->rs_rd) >> + || RelationIsAccessibleInLogicalDecoding(scan->rs_rd)) >> + heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin); >> + else >> + heap_page_prune_opt(scan->rs_rd, buffer, >> RecentGlobalDataXmin); >> >> Instead of changing the callers of heap_page_prune_opt() in this way, >> I think it might be better to change heap_page_prune_opt() to take >> only the first two of its current three parameters; everybody's just >> passing RecentGlobalXmin right now anyway. > > I've changed stuff this way, and it indeed looks better. > > I am wondering about the related situation of GetOldestXmin() > callers. There's a fair bit of duplicated logic in the callers, before > but especially after this patchset. What about adding 'Relation rel' > parameter instead of `allDbs' and `systable'? That keeps the logic > centralized and there's been a fair amount of talk about vacuum > optimizations that could also use it. > It's a bit sad that that requires including rel.h from procarray.h... > > What do you think? Isolated patch attached.
Seems reasonable to me. + * considered, but for non-shared non-shared relations that's not required, Duplicate word. -- 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
