Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options
Date
Msg-id CALj2ACVs3N0X1-K8SULeQQ8=J44z7Vhg_fQMwTD4OL+MPheK9Q@mail.gmail.com
Whole thread Raw
In response to Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options  (Peter Smith <smithpb2250@gmail.com>)
Responses Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
On Sat, May 22, 2021 at 6:32 AM Peter Smith <smithpb2250@gmail.com> wrote:
> I am unsure if this will lead to better code or not; Anyway, it is
> something to consider - maybe you can experiment with it to see.

Thanks. I think using bitmaps would help us have clean code. This is
also more extensible. See pseudo code at [1]. One disadvantage is that
we might have bms_XXXfunction calls, but that's okay and it shouldn't
add too much to the performance. Thoughts?

[1]
typedef enum SubOpts_enum
{
SUB_OPT_NONE = 0,
SUB_OPT_CONNECT,
SUB_OPT_ENABLED,
SUB_OPT_CREATE_SLOT,
SUB_OPT_SLOT_NAME,
SUB_OPT_COPY_DATA,
SUB_OPT_SYNCHRONOUS_COMMIT,
SUB_OPT_REFRESH,
SUB_OPT_BINARY,
SUB_OPT_STREAMING
} SubOpts_enum;

typedef struct SubOptsVals
{
bool connect;
bool enabled;
bool create_slot;
char *slot_name;
bool copy_data;
char *synchronous_commit;
bool refresh;
bool binary;
bool streaming;
} SubOptsVals;

Bitmapset  *supported = NULL;
Bitmapset  *specified = NULL;
ParsedSubOpts opts;

MemSet(opts, 0, sizeof(ParsedSubOpts));
/* Fill in all the supported options, we could use bms_add_member as
well if there are less number of supported options.*/
supported = bms_add_range(NULL, SUB_OPT_CONNECT, SUB_OPT_STREAMING);
supported = bms_del_member(supported, SUB_OPT_REFRESH);

parse_subscription_options(stmt_options, supported, specified, &opts);

if (bms_is_member(SUB_OPT_SLOT_NAME, specified))
{
    /* get slot name with opts.slot_name */
}

if (bms_is_member(SUB_OPT_SYNCHRONOUS_COMMIT, specified))
{
     /* get slot name with opts.synchronous_commit */
}

/* Similarly get the other options. */

bms_free(supported);
bms_free(specified);

static void
parse_subscription_options(List *stmt_options,
                Bitmapset *supported,
                Bimapset *specified,
                SubOptsVals *opts)
{

    foreach(lc, stmt_options)
    {
        DefElem    *defel = (DefElem *) lfirst(lc);

        if (bms_is_member(SUB_OPT_CONNECT, supported) &&
            strcmp(defel->defname, "connect") == 0)
        {
            if (bms_is_member(SUB_OPT_CONNECT, specified))
                ereport(ERROR,
                        (errcode(ERRCODE_SYNTAX_ERROR),
                         errmsg("conflicting or redundant options")));

            specified = bms_add_member(specified, SUB_OPT_CONNECT);
            opts->connect = defGetBoolean(defel);
        }

        /* Similarly do the same for the other options. */
    }
}

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Michael Banck
Date:
Subject: Re: Commitfest app vs. pgsql-docs
Next
From: Eugen Konkov
Date:
Subject: Proposition for columns expanding: table_name.**