On Wed, Mar 25, 2026 at 7:29 PM Tomas Vondra <[email protected]> wrote: > > >> - Do we really want to pass two sets of flags to table_beginscan_common? > >> I realize it's done to ensure "users" don't use internal flags, but > >> then maybe it'd be better to do that check in the places calling the > >> _common? Someone adding a new caller can break this in various ways > >> anyway, e.g. by setting bits in the internal flags, no? > > > > Yes, callers of table_beginscan_common() could pass flags they > > shouldn't in internal_flags. But I was mostly trying to prevent the > > case where a user picks a flag that overlaps with an internal flag, > > conditionally passes it as a user flag, and then when they test for it > > in their AM-specific code, they aren't actually checking if their own > > flag is set. > > Ah, so we expect people to invent their "own" flags, outside what's in > ScanOptions? Or do I misunderstand how it works? (I admit not reading > the whole massive thread, as I was only interested in using the flags in > my own patch.)
Yes, this isn't really explored in the rest of the thread. I thought since the flags are threaded all the way through and they can set/check the flags in the table AM-specific layer, it would make sense that they could choose flags for their own purposes. They don't have to wait for consensus on getting a new SO type added. I don't know if this is a bad idea. However, changing the table AM wrappers seems more justifiable if we are making them extensible in this way. > >> If we want to have these checks, should we be more thorough? Should we > >> check the internal flags only set internal flags? > > > > That's easy enough too. > > Assert((internal_flags & ~SO_INTERNAL_FLAGS) == 0); I think does the trick. I did this in the previously posted v46. - Melanie
