On Sat, Sep 13, 2014 at 11:02 PM, Fabrízio de Royes Mello <fabriziome...@gmail.com> wrote: > Patch rebased and added to commitfest [1]. It looks like a good thing to remove ATChangeIndexesPersistence, this puts the persistence switch directly into reindex process.
A couple of minor comments about this patch: 1) Reading it, I am wondering if it would not be finally time to switch to a macro to get a relation's persistence, something like RelationGetPersistence in rel.h... Not related directly to this patch. 2) reindex_index has as new argument a relpersislence value for the new index. reindex_relation has differently a new set of flags to enforce the relpersistence of all the underling indexes. Wouldn't it be better for API consistency to pass directly a relpersistence value through reindex_relation? In any case, the comment block of reindex_relation does not contain a description of the new flags. 3) Here you may as well just set the value and be done: + /* + * Check if need to set the new relpersistence + */ + if (iRel->rd_rel->relpersistence != relpersistence) + iRel->rd_rel->relpersistence = relpersistence; Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
