On Wed, Dec 23, 2020 at 07:07:54PM -0600, Justin Pryzby wrote: > On Wed, Dec 23, 2020 at 09:14:18PM -0300, Alvaro Herrera wrote: >> On 2020-Dec-23, Justin Pryzby wrote: >>> This was getting ugly: >>> >>> extern void reindex_index(Oid indexId, bool skip_constraint_checks, >>> char relpersistence, int options, Oid >>> tablespaceOid) >> >> Is this what I suggested?
No idea if this is what you suggested, but it would be annoying to have to change this routine's signature each time we need to pass down a new option. > No ; that was from an earlier revision of the patch adding the tablespace, > before Michael suggested a ReindexOptions struct, which subsumes 'options' and > 'tablespaceOid'. > > I see now that 'skip_constraint_checks' is from REINDEX_REL_CHECK_CONSTRAINTS. > It seems like that should be a REINDEXOPT_* flag, rather than REINDEX_REL_*, > so doesn't need to be a separate boolean. See also: 2d3320d3d. FWIW, it still makes the most sense to me to keep the options that are extracted from the grammar or things that apply to all the sub-routines of REINDEX to be tracked in a single structure, so this should include only the REINDEXOPT_* set for now, with the tablespace OID as of this thread, and also the reindex filtering options. REINDEX_REL_* is in my opinion of a different family because they only apply to reindex_relation(), and partially to reindex_index(), so they are very localized. In short, anything in need of only reindex_relation() has no need to know about the whole ReindexOption business. -- Michael
signature.asc
Description: PGP signature
