On 2020-Dec-23, Michael Paquier wrote: > bool > -reindex_relation(Oid relid, int flags, int options) > +reindex_relation(Oid relid, int flags, ReindexOptions *options) > { > Relation rel; > Oid toast_relid;
Wait a minute. reindex_relation has 'flags' and *also* 'options' with an embedded 'flags' member? Surely that's not right. I see that they carry orthogonal sets of options ... but why aren't they a single bitmask instead of two separate ones? This looks weird and confusing. Also: it seems a bit weird to me to put the flags inside the options struct. I would keep them separate -- so initially the options struct would only have the tablespace OID, on API cleanliness grounds: struct ReindexOptions { tablepaceOid oid; }; extern bool reindex_relation(Oid relid, bits32 flags, ReindexOptions *options); I guess you could argue that it's more performance to set up only two arguments to the function call instead of three .. but I doubt that's measurable for anything in DDL-land. But also, are we really envisioning that these routines would have all that many additional options? Maybe it is sufficient to do just extern bool reindex_relation(Oid relid, bits32 flags, tablespaceOid Oid);