Re: Refactor to make use of a common function for GetSubscriptionRelations and GetSubscriptionNotReadyRelations.

On Wed, Jul 20, 2022 at 02:32:44PM +0530, Amit Kapila wrote:
> On Thu, Jul 14, 2022 at 4:34 AM Peter Smith  wrote:
>> By using a bitmask I think there is an implication that the flags can
>> be combined...
>>
>> Perhaps it is not a problem today, but later you may want more flags. e.g.
>> #define SUBSCRIPTION_REL_STATE_READY 0x02 /* READY relations */
>>
>> then the bitmask idea falls apart because IIUC you have no intentions
>> to permit things like:
>> (SUBSCRIPTION_REL_STATE_NOT_READY | SUBSCRIPTION_REL_STATE_READY)
> 
> I think this will be an invalid combination if caller ever used it.
> However, one might need to use a combination like
> (SUBSCRIPTION_REL_STATE_READY | SUBSCRIPTION_REL_STATE_DONE). For such
> cases, I feel the bitmask idea will be better.

It feels unnatural to me to have a flag saying "not-ready" and one
saying "ready", while we could have a flag saying "ready" that can be
combined with a second flag to decide if the contents of srsubstate
should be matched or *not* matched with the states expected by the
caller.  This could be extended to more state values, for example.

I am not sure if we actually need this much as I have no idea if
future features would use it, so please take my suggestion lightly :)
--
Michael