On Wed, Apr 29, 2026 at 11:50 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, Apr 28, 2026 at 7:53 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > 2.
> > > +typedef enum ConflictLogDest
> > > +{
> > > + /* Log conflicts to the server logs */
> > > + CONFLICT_LOG_DEST_LOG = 1 << 0, /* 0x01 */
> > > +
> > > + /* Log conflicts to an internally managed conflict log table */
> > > + CONFLICT_LOG_DEST_TABLE = 1 << 1, /* 0x02 */
> > > +
> > > + /* Convenience bitmask for all supported destinations */
> > > + CONFLICT_LOG_DEST_ALL = (CONFLICT_LOG_DEST_LOG | CONFLICT_LOG_DEST_TABLE)
> > > +} ConflictLogDest;
> > > +
> > > +/*
> > > + * Array mapping for converting internal enum to string.
> > > + */
> > > +static const char *const ConflictLogDestNames[] = {
> > > + [CONFLICT_LOG_DEST_LOG] = "log",
> > > + [CONFLICT_LOG_DEST_TABLE] = "table",
> > > + [CONFLICT_LOG_DEST_ALL] = "all"
> > > +};
> > >
> > > Defining an array this way could be an Array size issue. Actually the
> > > array has just three elements so the last element should be at
> > > ConflictLogDestNames[2] but if we go by the above definition, it will
> > > be ConflictLogDestNames[3]. Can we define by referring the following
> > > existing way:
>
> I was analyzing this because I remember we were initially using the
> format you suggested and switched to the bit format to enable direct
> bitwise operations elsewhere. I think Peter suggested that [1], and
> the argument was that the bitwise operation is easy if we represent
> them as a bit. Also, since we would not have too many options, the
> array size shouldn't be an issue. But I understand your point: adding
> more elements will cause the array size to grow very fast as this is
> using sparse array. Let's see what others think about this, and then
> we can decide whether to change it back?
>
The benefit of the current approach is that checking whether the
destination is TABLE becomes straightforward:
IsSet(opts.conflictlogdest,CONFLICT_LOG_DEST_TABLE)
if we go by regular enum values (simialr to XLogSource), then it will be:
if (opts.logdest == CONFLICT_LOG_DEST_TABLE ||
opts.logdest == CONFLICT_LOG_DEST_ALL)
For ease of extending the enum and its corresponding text mappings, my
personal preference is still the regular (non-bitwise) enum approach.
But if we anticipate adding more destination options in the future
that would be covered by ALL, checking for those in code could lead to
growing chains of OR conditions, whereas the bitwise approach scales
more cleanly in that respect. So I think the choice depends on what
kinds of future extensions we expect.
Do we have plans to add more options that would naturally fall under
ALL? Or do we instead expect additions that are mutually exclusive;
for example, splitting CONFLICT_LOG_DEST_LOG into something like
CONFLICT_LOG_DEST_JSON_LOG and CONFLICT_LOG_DEST_TEXT_LOG, which may
not make sense to group under ALL in the same way?
thanks
Shveta
