On Thu, Aug 24, 2017 at 12:38 PM, Bossart, Nathan <bossa...@amazon.com> wrote: > On 8/18/17, 12:56 AM, "Michael Paquier" <michael.paqu...@gmail.com> wrote: > According to the docs, VACUUM and ANALYZE "do not support recursing over > inheritance hierarchies" [1]. However, we need a list of OIDs for > partitioned tables. Namely, this piece of code in get_rel_oids(...): > > if (include_parts) > oid_list = list_concat(oid_list, > > find_all_inheritors(relid, NoLock, NULL)); > else > oid_list = lappend_oid(oid_list, relid); > > Since all of these OIDs should correspond to the same partitioned table, > it makes sense to me to leave them together instead of breaking out each > partition into a VacuumRelation. If we did so, I think we would also need > to duplicate the va_cols list for each partition. What do you think?
Robert, Amit and other folks working on extending the existing partitioning facility would be in better position to answer that, but I would think that we should have something as flexible as possible, and storing a list of relation OID in each VacuumRelation makes it harder to track the uniqueness of relations vacuumed. I agree that the concept of a partition with multiple parents induces a lot of problems. But the patch as proposed worries me as it complicates vacuum() with a double loop: one for each relation vacuumed, and one inside it with the list of OIDs. Modules calling vacuum() could also use flexibility, being able to analyze a custom list of columns for each relation has value as well. >> - * relid, if not InvalidOid, indicate the relation to process; otherwise, >> - * the RangeVar is used. (The latter must always be passed, because it's >> - * used for error messages.) >> + * If we intend to process all relations, the 'relations' argument may be >> + * NIL. >> This comment actually applies to RelationAndColumns. If the OID is >> invalid, then RangeVar is used, and should always be set. You are >> breaking that promise actually for autovacuum. The comment here should >> say that if relations is NIL all the relations of the database are >> processes, and for an ANALYZE all the columns are done. > > Makes sense, I've tried to make this comment clearer. + * relations is a list of VacuumRelations to process. If it is NIL, all + * relations in the database are processed. Typo here, VacuumRelation is singular. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
