On Thu, Jul 21, 2022 at 10:11 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jul 21, 2022 at 10:03 AM Michael Paquier <mich...@paquier.xyz> wrote: > > > > On Thu, Jul 21, 2022 at 09:54:05AM +0530, Amit Kapila wrote: > > > Yeah, it is not very clear to me either. I think this won't be > > > difficult to change one or another way depending on future needs. At > > > this stage, it appeared to me that bitmask is a better way to > > > represent this information but if you and other feels using enum is a > > > better idea then I am fine with that as well. > > > > Please don't get me wrong :) > > > > I favor a bitmask over an enum here, as you do, with a clean > > layer for those flags. > > > > Okay, let's see what Peter Smith has to say about this as he was in > favor of using enum here? >
I was in favour of enum mostly because I thought the bitmask of an earlier patch was mis-used; IMO each bit should only be for representing something as "on/set". So a bit for SUBSCRIPTION_REL_STATE_READY makes sense, but a bit for SUBSCRIPTION_REL_STATE_NOT_READY seemed strange/backwards to me. YMMV. So using a bitmask is fine, except I thought it should be implemented so that one of the bits is for a "NOT" modifier (IIUC this is kind of similar to what Michael [1] suggested above?). So "Not READY" would be (SUBSCRIPTION_REL_STATE_MOD_NOT | SUBSCRIPTION_REL_STATE_READY) Also, it may be better to add the bit constants for every one of the current states, even if you are not needing to use all of them just yet. In fact, I thought this patch probably can implement the fully capable common function (i.e. capable of multiple keys etc) right now, so there will be no need to revisit it again in the future. ------ [1] https://www.postgresql.org/message-id/Ytiuj4hLykTvBF46%40paquier.xyz Kind Regards, Peter Smith. Fujitsu Australia
