> Thanks for the updated patch, few comments:
> 1) The option name seems wrong here:
> In one place include_generated_column is specified and other place
> include_generated_columns is specified:
>
> + else if (IsSet(supported_opts,
> SUBOPT_INCLUDE_GENERATED_COLUMN) &&
> + strcmp(defel->defname,
> "include_generated_column") == 0)
> + {
> + if (IsSet(opts->specified_opts,
> SUBOPT_INCLUDE_GENERATED_COLUMN))
> + errorConflictingDefElem(defel, pstate);
> +
> + opts->specified_opts |=
> SUBOPT_INCLUDE_GENERATED_COLUMN;
> + opts->include_generated_column = defGetBoolean(defel);
> + }
Fixed.
> diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
> index d453e224d9..e8ff752fd9 100644
> --- a/src/bin/psql/tab-complete.c
> +++ b/src/bin/psql/tab-complete.c
> @@ -3365,7 +3365,7 @@ psql_completion(const char *text, int start, int end)
> COMPLETE_WITH("binary", "connect", "copy_data", "create_slot",
> "disable_on_error",
> "enabled", "failover", "origin",
> "password_required",
> "run_as_owner", "slot_name",
> - "streaming",
> "synchronous_commit", "two_phase");
> + "streaming",
> "synchronous_commit", "two_phase","include_generated_columns");
>
> 2) This small data table need not have a primary key column as it will
> create an index and insertion will happen in the index too.
> +-- check include-generated-columns option with generated column
> +CREATE TABLE gencoltable (a int PRIMARY KEY, b int GENERATED ALWAYS
> AS (a * 2) STORED);
> +INSERT INTO gencoltable (a) VALUES (1), (2), (3);
> +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
> NULL, 'include-xids', '0', 'skip-empty-xacts', '1',
> 'include-generated-columns', '1');
Fixed.
> 3) Please add a test case for this:
> + set to <literal>false</literal>. If the subscriber-side
> column is also a
> + generated column then this option has no effect; the
> replicated data will
> + be ignored and the subscriber column will be filled as
> normal with the
> + subscriber-side computed or default data.
Added the required test case.
> 4) You can use a new style of ereport to remove the brackets around errcode
> 4.a)
> + else if (!parse_bool(strVal(elem->arg),
> &data->include_generated_columns))
> + ereport(ERROR,
> +
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("could not
> parse value \"%s\" for parameter \"%s\"",
> +
> strVal(elem->arg), elem->defname)));
>
> 4.b) similarly here too:
> + ereport(ERROR,
> + (errcode(ERRCODE_SYNTAX_ERROR),
> + /*- translator: both %s are strings of the form
> "option = value" */
> + errmsg("%s and %s are mutually
> exclusive options",
> + "copy_data = true",
> "include_generated_column = true")));
>
> 4.c) similarly here too:
> + if (include_generated_columns_option_given)
> + ereport(ERROR,
> +
> (errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("conflicting
> or redundant options")));
Fixed.
> 5) These variable names can be changed to keep it smaller, something
> like gencol or generatedcol or gencolumn, etc
> +++ b/src/include/catalog/pg_subscription.h
> @@ -98,6 +98,8 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId)
> BKI_SHARED_RELATION BKI_ROW
> * slots) in the upstream database are enabled
> * to be synchronized to the standbys. */
>
> + bool subincludegeneratedcolumn; /* True if generated columns must be
> published */
> +
> #ifdef CATALOG_VARLEN /* variable-length fields start here */
> /* Connection string to the publisher */
> text subconninfo BKI_FORCE_NOT_NULL;
> @@ -157,6 +159,7 @@ typedef struct Subscription
> List *publications; /* List of publication names to subscribe to */
> char *origin; /* Only publish data originating from the
> * specified origin */
> + bool includegeneratedcolumn; /* publish generated column data */
> } Subscription;
Fixed.
The attached Patch contains the suggested changes.
Thanks and Regards,
Shubham Khanna.
v6-0001-Enable-support-for-include_generated_columns-opti.patch
Description: Binary data
