RE: why can't a table be part of the same publication as its schema

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Subject: RE: why can't a table be part of the same publication as its schema
Date: 2022-09-16 07:39:42
Message-ID: OS0PR01MB57169D60C2EFF8606EC10F8094489@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Friday, September 16, 2022 1:42 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Sep 15, 2022 at 6:27 PM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Attach the new version patch which added suggested restriction for
> > column list and merged Vignesh's patch.
> >
>
> Few comments:
> ============
> 1.
> static void
> -CheckPubRelationColumnList(List *tables, const char *queryString,
> +CheckPubRelationColumnList(List *tables, bool publish_schema,
> + const char *queryString,
> bool pubviaroot)
>
> It is better to keep bool parameters together at the end.
>
> 2.
> /*
> + * Disallow using column list if any schema is in the publication.
> + */
> + if (publish_schema)
> + ereport(ERROR,
> + errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("cannot use publication column list for relation \"%s.%s\"",
> + get_namespace_name(RelationGetNamespace(pri->relation)),
> + RelationGetRelationName(pri->relation)),
> + errdetail("Column list cannot be specified if any schema is part of
> the publication or specified in the list."));
>
> I think it would be better to explain why we disallow this case.
>
> 3.
> + if (!heap_attisnull(coltuple, Anum_pg_publication_rel_prattrs, NULL))
> + ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("cannot add schema to the publication"), errdetail("Schema
> + cannot be added if any table that specifies column
> list is already part of the publication"));
>
> A full stop is missing at the end in the errdetail message.
>
> 4. I have modified a few comments in the attached. Please check and if you like
> the changes then please include those in the next version.

Thanks for the comments.
Attach the new version patch which addressed above comments and ran pgident.
I also improved some codes and documents based on some comments
given by Vignesh and Peter offlist.

Best regards,
Hou zj

Attachment Content-Type Size
v4-0001-Allow-publications-with-schema-and-table-of-the-s.patch application/octet-stream 34.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2022-09-16 07:54:14 Re: [PoC] Improve dead tuple storage for lazy vacuum
Previous Message Marina Polyakova 2022-09-16 07:31:42 Re: ICU for global collation