On Sat, May 22, 2021 at 6:32 AM Peter Smith <smithpb2...@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