On Sat, Dec 12, 2020 at 01:45:26PM -0600, Justin Pryzby wrote: > On Sat, Dec 12, 2020 at 09:20:35AM +0100, Peter Eisentraut wrote: >> On 2020-12-11 21:27, Alvaro Herrera wrote: >>> By the way-- What did you think of the idea of explictly marking the >>> types used for bitmasks using types bits32 and friends, instead of plain >>> int, which is harder to spot? >> >> If we want to make it clearer, why not turn the thing into a struct, as in >> the attached patch, and avoid the bit fiddling altogether.
- reindex_relation(OIDOldHeap, reindex_flags, 0); + reindex_relation(OIDOldHeap, reindex_flags, (ReindexOptions){}); This is not common style in the PG code. Usually we would do that with memset(0) or similar. + bool REINDEXOPT_VERBOSE; /* print progress info */ + bool REINDEXOPT_REPORT_PROGRESS; /* report pgstat progress */ + bool REINDEXOPT_MISSING_OK; /* skip missing relations */ + bool REINDEXOPT_CONCURRENTLY; /* concurrent mode */ +} ReindexOptions Neither is this one to use upper-case characters for variable names. Now, we will need a ReindexOptions in the long-term to store all those options and there would be much more coming that booleans here (this thread talks about tablespaces, there is another thread about collation filtering). Between using bits32 with some hex flags or just a set of booleans within a structure, I would choose the former as a matter of habit but yours has the advantage to make debugging a no-brainer, which is good. For any approach taken, it seems to me that the same style should be applied to ClusterOption and Vacuum{Option,Params}. -- Michael
signature.asc
Description: PGP signature