On Fri, Dec 10, 2021 at 6:24 AM Alvaro Herrera wrote:
>
> I just noticed that this (commit 5a2832465fd8) added a separate catalog
> to store schemas which are part of a publication, side-by-side with the
> catalog to store relations which are part of a publication. This seems
> a strange way to
On Fri, Dec 10, 2021 at 6:24 AM Alvaro Herrera wrote:
>
> I just noticed that this (commit 5a2832465fd8) added a separate catalog
> to store schemas which are part of a publication, side-by-side with the
> catalog to store relations which are part of a publication. This seems
> a strange way to
I just noticed that this (commit 5a2832465fd8) added a separate catalog
to store schemas which are part of a publication, side-by-side with the
catalog to store relations which are part of a publication. This seems
a strange way to represent publication membership: in order to find out
what
On Tue, Nov 9, 2021 at 2:51 PM Amit Kapila wrote:
>
> On Tue, Nov 9, 2021 at 7:20 AM Peter Smith wrote:
> >
> > FYI - I spotted a trivial SQL mistake (?) of the schema publication patch
> > [1].
> >
> > See the file describe.c, function describeOneTableDetails.
> > The new SQL has a 3rd UNION
On Tue, Nov 9, 2021 at 7:20 AM Peter Smith wrote:
>
> FYI - I spotted a trivial SQL mistake (?) of the schema publication patch
> [1].
>
> See the file describe.c, function describeOneTableDetails.
> The new SQL has a 3rd UNION that looks like:
>
> ...
> "UNION\n"
> "SELECT pubname\n"
> "FROM
FYI - I spotted a trivial SQL mistake (?) of the schema publication patch [1].
See the file describe.c, function describeOneTableDetails.
The new SQL has a 3rd UNION that looks like:
...
"UNION\n"
"SELECT pubname\n"
"FROM pg_catalog.pg_publication p\n"
"WHERE puballtables AND
> On Thu, Nov 4, 2021 at 3:24 PM vignesh C wrote:
> >
> > On Thu, Nov 4, 2021 at 5:41 AM Peter Smith
> wrote:
> > >
> > > FYI - I found a small problem with one of the new PublicationObjSpec
> > > parser error messages that was introduced by the recent schema
> > > publication commit [1].
> > >
On Thu, Nov 4, 2021 at 3:24 PM vignesh C wrote:
>
> On Thu, Nov 4, 2021 at 5:41 AM Peter Smith wrote:
> >
> > FYI - I found a small problem with one of the new PublicationObjSpec
> > parser error messages that was introduced by the recent schema
> > publication commit [1].
> >
> > The error
On Thu, Nov 4, 2021 at 5:41 AM Peter Smith wrote:
>
> FYI - I found a small problem with one of the new PublicationObjSpec
> parser error messages that was introduced by the recent schema
> publication commit [1].
>
> The error message text is assuming that the error originates from
> CREATE
On Thu, Nov 4, 2021 at 11:55 AM houzj.f...@fujitsu.com
wrote:
>
> On Thurs, Nov 4, 2021 8:12 AM Peter Smith wrote:
> > FYI - I found a small problem with one of the new PublicationObjSpec parser
> > error messages that was introduced by the recent schema publication commit
> > [1].
> >
> > The
On Thurs, Nov 4, 2021 8:12 AM Peter Smith wrote:
> FYI - I found a small problem with one of the new PublicationObjSpec parser
> error messages that was introduced by the recent schema publication commit
> [1].
>
> The error message text is assuming that the error originates from CREATE
>
FYI - I found a small problem with one of the new PublicationObjSpec
parser error messages that was introduced by the recent schema
publication commit [1].
The error message text is assuming that the error originates from
CREATE PUBLICATION, but actually that same error can also come from
ALTER
On Wed, Nov 3, 2021 at 11:55 AM vignesh C wrote:
>
> On Wed, Nov 3, 2021 at 11:37 AM Peter Smith wrote:
> >
> > While you are changing these, maybe also change:
> >
> > Before: PUBLICATIONOBJ..._CURRSCHEMA
> > After: PUBLICATIONOBJ..._CUR_SCHEMA
> >
> > Which seems more readable to me.
>
>
On Wed, Nov 3, 2021 at 11:07 AM houzj.f...@fujitsu.com
wrote:
>
> On Wed, Nov 3, 2021 12:25 PM vignesh C wrote:
> > On Wed, Nov 3, 2021 at 8:30 AM Amit Kapila
> > wrote:
> > >
> > > On Tue, Nov 2, 2021 at 8:13 PM Tomas Vondra
> > > wrote:
> > > >
> > > >
> > > > >
> > > > > Yeah, that is also
On Wed, Nov 3, 2021 at 11:37 AM Peter Smith wrote:
>
> While you are changing these, maybe also change:
>
> Before: PUBLICATIONOBJ..._CURRSCHEMA
> After: PUBLICATIONOBJ..._CUR_SCHEMA
>
> Which seems more readable to me.
Thanks for the comment, the attached patch has the changes for the same.
While you are changing these, maybe also change:
Before: PUBLICATIONOBJ..._CURRSCHEMA
After: PUBLICATIONOBJ..._CUR_SCHEMA
Which seems more readable to me.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Nov 3, 2021 12:25 PM vignesh C wrote:
> On Wed, Nov 3, 2021 at 8:30 AM Amit Kapila
> wrote:
> >
> > On Tue, Nov 2, 2021 at 8:13 PM Tomas Vondra
> > wrote:
> > >
> > >
> > > >
> > > > Yeah, that is also true. So maybe at this, we can just rename the
> > > > few types as suggested by you
On Wed, Nov 3, 2021 at 8:30 AM Amit Kapila wrote:
>
> On Tue, Nov 2, 2021 at 8:13 PM Tomas Vondra
> wrote:
> >
> >
> > >
> > > Yeah, that is also true. So maybe at this, we can just rename the few
> > > types as suggested by you and we can look at it later if we anytime
> > > have more number of
On Tue, Nov 2, 2021 at 8:13 PM Tomas Vondra
wrote:
>
>
> >
> > Yeah, that is also true. So maybe at this, we can just rename the few
> > types as suggested by you and we can look at it later if we anytime
> > have more number of objects to add.
> >
>
> +1
>
Apart from what you have pointed
On 11/2/21 11:37 AM, Amit Kapila wrote:
> On Mon, Nov 1, 2021 at 5:52 PM Tomas Vondra
> wrote:
>>
>> On 11/1/21 11:18, Amit Kapila wrote:
>>> On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
>>> wrote:
I wonder if it'd be better to just separate the schema and object type
specification,
On Mon, Nov 1, 2021 at 5:52 PM Tomas Vondra
wrote:
>
> On 11/1/21 11:18, Amit Kapila wrote:
> > On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
> > wrote:
> >> I wonder if it'd be better to just separate the schema and object type
> >> specification, instead of mashing it into a single constant.
>
On Tue, Nov 2, 2021 at 6:43 AM Masahiko Sawada wrote:
>
> > I am just not sure if it is worth adding
> > such a view or we leave it to users to find that information via
> > querying individual views or system tables for objects.
>
> I've not looked at the patch for logical replication of
On Mon, Nov 1, 2021 at 7:28 PM Amit Kapila wrote:
>
> On Mon, Nov 1, 2021 at 2:38 PM Greg Nancarrow wrote:
> >
> > On Mon, Nov 1, 2021 at 5:07 PM Masahiko Sawada
> > wrote:
> > >
> > > I haven't followed the discussion on pg_publication_objects view but
> > > what is the primary use case of
On 11/1/21 11:18, Amit Kapila wrote:
On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
wrote:
On 10/28/21 04:41, Amit Kapila wrote:
On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila wrote:
On Mon, Oct 25, 2021 at 1:11 PM vignesh C wrote:
I have fixed this in the v47 version attached.
Thanks,
On Mon, Nov 1, 2021 at 2:38 PM Greg Nancarrow wrote:
>
> On Mon, Nov 1, 2021 at 5:07 PM Masahiko Sawada wrote:
> >
> > I haven't followed the discussion on pg_publication_objects view but
> > what is the primary use case of this view? If it's to list all tables
> > published in a publication
On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
wrote:
>
> On 10/28/21 04:41, Amit Kapila wrote:
> > On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila wrote:
> >>
> >> On Mon, Oct 25, 2021 at 1:11 PM vignesh C wrote:
> >>>
> >>> I have fixed this in the v47 version attached.
> >>>
> >>
> >> Thanks, the
On Mon, Nov 1, 2021 at 5:07 PM Masahiko Sawada wrote:
>
> I haven't followed the discussion on pg_publication_objects view but
> what is the primary use case of this view? If it's to list all tables
> published in a publication (e.g, "select * from pg_publication_objects
> where pubname =
On Fri, Oct 29, 2021 at 1:35 PM Amit Kapila wrote:
>
> On Thu, Oct 28, 2021 at 9:55 AM vignesh C wrote:
> >
> > Thanks for committing the patch, please find the remaining patches attached.
> > Thanks Hou Zhijie and Greg Nancarrow for sharing a few comments
> > offline, I have fixed those in the
On Fri, Oct 29, 2021 at 3:35 PM Amit Kapila wrote:
>
> Sawada-San, others, what do you think? Is it really useful to have such a
> view?
>
> One point to think is if we introduce such a view then how it should
> behave for schema objects? Do we need to display only schemas or
> additionally all
Hi,
On 10/28/21 04:41, Amit Kapila wrote:
On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila wrote:
On Mon, Oct 25, 2021 at 1:11 PM vignesh C wrote:
I have fixed this in the v47 version attached.
Thanks, the first patch in the series "Allow publishing the tables of
schema." looks good to me.
On Friday, October 29, 2021 12:35 PM, Amit Kapila
wrote:
>
> On Thu, Oct 28, 2021 at 9:55 AM vignesh C wrote:
> >
> > Thanks for committing the patch, please find the remaining patches attached.
> > Thanks Hou Zhijie and Greg Nancarrow for sharing a few comments
> > offline, I have fixed those
On Thu, Oct 28, 2021 at 9:55 AM vignesh C wrote:
>
> Thanks for committing the patch, please find the remaining patches attached.
> Thanks Hou Zhijie and Greg Nancarrow for sharing a few comments
> offline, I have fixed those in the attached patch.
>
Pushed the first test case patch. About
On Thu, Oct 28, 2021 at 3:25 PM vignesh C wrote:
>
> Thanks for committing the patch, please find the remaining patches attached.
A few comments on the v48-0002 patch:
(1) The quoting of TABLE/SCHEMA looks a bit odd in the patch comment
(2) src/backend/catalog/system_views.sq
ON should be
On Thu, Oct 28, 2021 at 8:12 AM Amit Kapila wrote:
>
> On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila wrote:
> >
> > On Mon, Oct 25, 2021 at 1:11 PM vignesh C wrote:
> > >
> > > I have fixed this in the v47 version attached.
> > >
> >
> > Thanks, the first patch in the series "Allow publishing the
On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila wrote:
>
> On Mon, Oct 25, 2021 at 1:11 PM vignesh C wrote:
> >
> > I have fixed this in the v47 version attached.
> >
>
> Thanks, the first patch in the series "Allow publishing the tables of
> schema." looks good to me. Unless there are more
>
On Mon, Oct 25, 2021 at 1:11 PM vignesh C wrote:
>
> On Mon, Oct 25, 2021 at 10:52 AM Amit Kapila wrote:
> >
> > On Fri, Oct 22, 2021 at 8:56 PM vignesh C wrote:
> > >
> >
> > I am getting a compilation error in the latest patch on HEAD. I think
> > was relying on some variable removed by a
On Fri, Oct 22, 2021 at 8:56 PM vignesh C wrote:
>
I am getting a compilation error in the latest patch on HEAD. I think
was relying on some variable removed by a recent commit
92316a4582a5714d4e494aaf90360860e7fec37a. While looking at that
compilation error, I observed that we don't need the
On Fri, Oct 22, 2021 at 11:59 AM Greg Nancarrow wrote:
>
> On Fri, Oct 22, 2021 at 12:41 PM Greg Nancarrow wrote:
> >
> > I was also previously concerned about what the behavior should be when
> > only including just the partitions of a partitioned table in a
> > publication using ALL TABLES IN
On Fri, Oct 22, 2021 at 1:03 PM Masahiko Sawada wrote:
>
> On Fri, Oct 22, 2021 at 2:25 PM Amit Kapila wrote:
> >
> > On Thu, Oct 21, 2021 at 6:47 PM vignesh C wrote:
> > >
> > >
> > > Thanks for the comments, the attached v45 patch has the fix for the same.
> > >
> >
> > The first patch is
On Fri, Oct 22, 2021 at 11:59 AM Greg Nancarrow wrote:
>
> On Fri, Oct 22, 2021 at 12:41 PM Greg Nancarrow wrote:
> >
> > I was also previously concerned about what the behavior should be when
> > only including just the partitions of a partitioned table in a
> > publication using ALL TABLES IN
On Fri, Oct 22, 2021 at 2:25 PM Amit Kapila wrote:
>
> On Thu, Oct 21, 2021 at 6:47 PM vignesh C wrote:
> >
> >
> > Thanks for the comments, the attached v45 patch has the fix for the same.
> >
>
> The first patch is mostly looking good to me apart from the below
> minor comments:
Let me share
On Fri, Oct 22, 2021 at 12:41 PM Greg Nancarrow wrote:
>
> I was also previously concerned about what the behavior should be when
> only including just the partitions of a partitioned table in a
> publication using ALL TABLES IN SCHEMA and having
> publish_via_partition_root=true. It seems to
On Thu, Oct 21, 2021 at 6:47 PM vignesh C wrote:
>
>
> Thanks for the comments, the attached v45 patch has the fix for the same.
>
The first patch is mostly looking good to me apart from the below
minor comments:
1.
+
+ The catalog pg_publication_namespace contains the
+ mapping between
On Fri, Oct 22, 2021 at 12:19 AM vignesh C wrote:
>
> I could reproduce the issue by using the following test:
> --- Setup
> create schema sch1;
> create schema sch2;
> create table sch1.tbl1 (a int) partition by range (a);
> create table sch2.tbl1_part1 partition of sch1.tbl1 for values from (1)
On Thu, Oct 21, 2021 at 3:29 PM Greg Nancarrow wrote:
>
> On Thu, Oct 21, 2021 at 3:25 AM vignesh C wrote:
> >
> > Attached v44 patch as the fixes for the same.
> >
>
> In the v44-0001 patch, I have some doubts about the condition guarding
> the following code in pg_get_publication_tables():
>
>
On Thu, Oct 21, 2021 at 3:25 AM vignesh C wrote:
>
> Attached v44 patch as the fixes for the same.
>
In the v44-0001 patch, I have some doubts about the condition guarding
the following code in pg_get_publication_tables():
+ if (schemarelids)
+ {
+/*
+ * If the publication publishes
On Thurs, Oct 21, 2021 12:25 AM vignesh C wrote:
> This version of patch retains the changes related to PublicationRelInfo, I
> will
> handle the merging of the patches in the next version so that this version of
> patch change related to PublicationRelInfo can be reviewed easily.
Thanks for
On Thu, Oct 21, 2021 at 3:25 AM vignesh C wrote:
>
> Attached v44 patch as the fixes for the same.
>
Regarding the documentation, I think some minor updates are needed in:
doc/src/sgml/logical-replication.sgml.
For example, currently it says:
Publications may currently only contain tables.
On Tue, Oct 19, 2021 at 9:42 PM vignesh C wrote:
>
Thanks for the modified patch. I have a few more comments and suggestions:
As the thread [1] is still not concluded, I suggest we fix the
duplicate data case only when schemas are involved by slightly
tweaking the code as per attached. This is
On Tuesday, October 19, 2021 11:42 PM vignesh C wrote:
>
> This issue got induced in the v42 version, attached v43 patch has the
> fixes for the same.
>
Thanks for your new patch. I confirmed that this issue has be fixed.
All regression tests passed.
I also tested V43 in some other scenarios
On Tue, Oct 19, 2021 at 11:23 AM tanghy.f...@fujitsu.com
wrote:
>
> On Tuesday, October 19, 2021 12:57 PM Amit Kapila
> wrote:
> >
> > On Tue, Oct 19, 2021 at 9:15 AM tanghy.f...@fujitsu.com
> > wrote:
> > >
> > > On Monday, October 18, 2021 8:23 PM vignesh C
> > wrote:
> > > >
> > > > Thanks
On Mon, Oct 18, 2021 at 5:53 PM vignesh C wrote:
>
Few comments on latest set of patches:
===
1.
+/*
+ * Filter out the partitions whose parent tables was also specified in
+ * the publication.
+ */
+static List *
+filter_out_partitions(List *relids)
Can we name this
On Tuesday, October 19, 2021 12:57 PM Amit Kapila
wrote:
>
> On Tue, Oct 19, 2021 at 9:15 AM tanghy.f...@fujitsu.com
> wrote:
> >
> > On Monday, October 18, 2021 8:23 PM vignesh C
> wrote:
> > >
> > > Thanks for the comments, the attached v42 patch has the fixes for the
> > > same.
> >
> >
On Tue, Oct 19, 2021 at 9:15 AM tanghy.f...@fujitsu.com
wrote:
>
> On Monday, October 18, 2021 8:23 PM vignesh C wrote:
> >
> > Thanks for the comments, the attached v42 patch has the fixes for the same.
>
> Thanks for your new patch.
>
> I tried your patch and found that the permission check
On Monday, October 18, 2021 8:23 PM vignesh C wrote:
>
> Thanks for the comments, the attached v42 patch has the fixes for the same.
Thanks for your new patch.
I tried your patch and found that the permission check for superuser didn't
work.
For example:
postgres=# create role r1;
CREATE
Hi,
On Mon, Oct 18, 2021 at 3:14 PM houzj.f...@fujitsu.com
wrote:
>
> On Saturday, October 16, 2021 1:57 PM houzj.f...@fujitsu.com wrote:
> > Based on the V40 patchset, attaching the Top-up patch which try to fix the
> > partition issue in a cleaner way.
>
> Attach the new version patch set
On Monday, October 18, 2021 12:04 PM Greg Nancarrow wrote:
> On Sat, Oct 16, 2021 at 4:57 PM houzj.f...@fujitsu.com wrote:
> > Based on the V40 patchset, attaching the Top-up patch which try to fix
> > the partition issue in a cleaner way.
> >
>
> A minor thing, in your "top-up patch", the test
On Sat, Oct 16, 2021 at 4:57 PM houzj.f...@fujitsu.com
wrote:
>
> Besides, I found we misunderstood the flag PUBLICATION_PART_ROOT it means:
> "ROOT: only the table explicitly mentioned in the publication" We cannot use
> it
> as a flag to judge whether do the partition filtering, I think we
On Fri, Oct 15, 2021 at 6:45 AM Greg Nancarrow wrote:
>
> On Thu, Oct 14, 2021 at 9:59 PM Amit Kapila wrote:
> >
> > > If partitions belong to a different schema than the parent partitioned
> > > table, then the current patch implementation allows the partitions to
> > > (optionally) be
On Thu, Oct 14, 2021 at 9:59 PM Amit Kapila wrote:
>
> > If partitions belong to a different schema than the parent partitioned
> > table, then the current patch implementation allows the partitions to
> > (optionally) be explicitly added to a publication that includes the
> > parent partitioned
On Wed, Oct 13, 2021 at 1:40 PM Greg Nancarrow wrote:
>
> On Wed, Oct 13, 2021 at 12:15 AM vignesh C wrote:
> >
> > Attached v40 patch has the fix for the above comments.
> >
>
> [Maybe this has some overlap with what Hou-san reported, and I have
> not tested this against his proposed fixes]
>
>
On Wednesday, October 13, 2021 4:10 PM Greg Nancarrow
wrote:
> Also, I found the following scenario where the data is double-published:
>
> (1) PUB: CREATE PUBLICATION pub FOR TABLE sch1.sale_201901, TABLE
> sch1.sale_201902 WITH (publish_via_partition_root=true);
> (2) SUB: CREATE
On Wed, Oct 13, 2021 at 12:15 AM vignesh C wrote:
>
> Attached v40 patch has the fix for the above comments.
>
[Maybe this has some overlap with what Hou-san reported, and I have
not tested this against his proposed fixes]
If partitions belong to a different schema than the parent partitioned
On Tuesday, October 12, 2021 9:15 PM vignesh C
> Attached v40 patch has the fix for the above comments.
Thanks for the update, I have some minor issues about partition related
behavior.
1)
Tang tested and discussed this issue with me.
The testcase is:
We publish a schema and there is a
On Monday, October 11, 2021 11:02 PM vignesh C wrote:
> The attached v39 patch has the fixes for the above issues.
Thanks for the updates.
I have a few minor suggestions about the testcases in the v39-0003-Test patch.
1)
+-- alter publication drop CURRENT_SCHEMA
+ALTER PUBLICATION
On Mon, Oct 11, 2021 at 1:21 PM Greg Nancarrow wrote:
>
> On Mon, Oct 11, 2021 at 5:39 PM vignesh C wrote:
> >
> > These comments are fixed in the v38 patch attached.
> >
>
> Thanks for the updates.
> I noticed that these patches don't apply on the latest source (last
> seemed to apply cleanly
On Mon, Oct 11, 2021 at 5:39 PM vignesh C wrote:
>
> These comments are fixed in the v38 patch attached.
>
Thanks for the updates.
I noticed that these patches don't apply on the latest source (last
seemed to apply cleanly on HEAD as at about October 6).
Regards,
Greg Nancarrow
Fujitsu
On Monday, October 11, 2021 2:39 PM vignesh C wrote:
>
> These comments are fixed in the v38 patch attached.
Thanks for updating the patches.
Here are a few comments on the v38-0004-Doc patch.
1.
+
+ Adding/Setting a table that is part of schema specified in
+ ALL TABLES IN SCHEMA,
On Mon, Oct 11, 2021 at 7:46 AM tanghy.f...@fujitsu.com
wrote:
>
> > On Friday, October 8, 2021 7:05 PM Amit Kapila
> > wrote:
> >
> > v37-0003-Tests-for-FOR-ALL-TABLES-IN-SCHEMA-publication
> > 3.
> > --- a/src/bin/pg_dump/t/002_pg_dump.pl
> > +++ b/src/bin/pg_dump/t/002_pg_dump.pl
> > ..
> >
On Fri, Oct 8, 2021 at 4:34 PM Amit Kapila wrote:
>
> On Thu, Oct 7, 2021 at 5:19 PM Amit Kapila wrote:
> >
> > On Wed, Oct 6, 2021 at 11:12 AM vignesh C wrote:
> > >
> > > Attached v37 patch has the changes for the same.
> > >
> >
> > Few comments:
> > ==
> >
>
> Few more comments:
> On Friday, October 8, 2021 7:05 PM Amit Kapila
> wrote:
>
> v37-0003-Tests-for-FOR-ALL-TABLES-IN-SCHEMA-publication
> 3.
> --- a/src/bin/pg_dump/t/002_pg_dump.pl
> +++ b/src/bin/pg_dump/t/002_pg_dump.pl
> ..
> + 'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test' => {
> + create_order
On Thu, Oct 7, 2021 at 5:19 PM Amit Kapila wrote:
>
> On Wed, Oct 6, 2021 at 11:12 AM vignesh C wrote:
> >
> > Attached v37 patch has the changes for the same.
> >
>
> Few comments:
> ==
>
Few more comments:
On Wed, Oct 6, 2021 at 11:12 AM vignesh C wrote:
>
> Attached v37 patch has the changes for the same.
>
Few comments:
==
v37-0001-Added-schema-level-support-for-publication
1.
+ *
+ * The first scan will get all the 'r' relkind tables for the specified schema,
+ * itera
On Wed, Oct 6, 2021 at 4:42 PM vignesh C wrote:
>
> Attached v37 patch has the changes for the same.
>
A small issue I noticed is that using "\dS" in PSQL shows UNLOGGED
tables as belonging to a publication, if the table belongs to a schema
that was added to the publication using ALL TABLES IN
On Tue, Oct 5, 2021 at 6:57 AM Greg Nancarrow wrote:
>
> On Mon, Oct 4, 2021 at 4:55 AM vignesh C wrote:
> >
> > Attached v36 patch has the changes for the same.
> >
>
> I have some comments on the v36-0001 patch:
>
> src/backend/commands/publicationcmds.c
>
> (1)
> GetPublicationSchemas()
>
> +
On Tue, Oct 5, 2021 at 4:41 PM Amit Kapila wrote:
>
> On Tue, Oct 5, 2021 at 11:10 AM Greg Nancarrow wrote:
> >
> > On Tue, Oct 5, 2021 at 3:11 PM Amit Kapila wrote:
> > >
> > > > Code has been added to prevent a table being set (via ALTER TABLE) to
> > > > UNLOGGED if it is part of a
On Tue, Oct 5, 2021 at 11:10 AM Greg Nancarrow wrote:
>
> On Tue, Oct 5, 2021 at 3:11 PM Amit Kapila wrote:
> >
> > > Code has been added to prevent a table being set (via ALTER TABLE) to
> > > UNLOGGED if it is part of a publication, but I found that I could
> > > still add all tables of a
On Tue, Oct 5, 2021 at 4:40 PM Greg Nancarrow wrote:
>
>At the
> moment, the existing documentation just states FOR TABLE that
> "Temporary tables, unlogged tables, foreign tables, materialized
> views, and regular views cannot be part of a publication".
Oh, I see that in the v36-0004 doc update
On Tue, Oct 5, 2021 at 3:11 PM Amit Kapila wrote:
>
> > Code has been added to prevent a table being set (via ALTER TABLE) to
> > UNLOGGED if it is part of a publication, but I found that I could
> > still add all tables of a schema having a table that is UNLOGGED:
> >
> > postgres=# create
On Tue, Oct 5, 2021 at 6:57 AM Greg Nancarrow wrote:
>
> (5)
>
> Code has been added to prevent a table being set (via ALTER TABLE) to
> UNLOGGED if it is part of a publication, but I found that I could
> still add all tables of a schema having a table that is UNLOGGED:
>
> postgres=# create
On Mon, Oct 4, 2021 at 4:55 AM vignesh C wrote:
>
> Attached v36 patch has the changes for the same.
>
I have some comments on the v36-0001 patch:
src/backend/commands/publicationcmds.c
(1)
GetPublicationSchemas()
+ /* Find all publications associated with the schema */
+ pubschsrel =
On Sun, Oct 3, 2021 at 11:25 PM vignesh C wrote:
>
> On Sat, Oct 2, 2021 at 1:13 PM Amit Kapila wrote:
> >
>
> > 2. In GetSchemaPublicationRelations(), I think we need to perform a
> > second scan using RELKIND_PARTITIONED_TABLE only if we
> > publish_via_root (aka pub_partopt is
On Thu, Sep 30, 2021 at 3:39 PM vignesh C wrote:
>
> The suggested change works, I have modified it in the attached patch.
>
I have reviewed the latest version and made a number of changes to the
0001 patch. The changes are in v1-0001-Changes-by-Amit. It includes
(a) Changed
On Wed, Sep 29, 2021 at 11:49 AM Greg Nancarrow wrote:
>
> On Wed, Sep 29, 2021 at 3:16 PM Amit Kapila wrote:
> >
> > 4.
> > + /*
> > + * Check if setting the relation to a different schema will result in the
> > + * publication having schema and same schema's table in the publication.
> > + */
On Wed, Sep 29, 2021 at 10:46 AM Amit Kapila wrote:
>
> On Tue, Sep 28, 2021 at 8:15 PM vignesh C wrote:
> >
> > On Mon, Sep 27, 2021 at 12:15 PM houzj.f...@fujitsu.com
> > wrote:
> >
> > Attached v34 patch has the changes for the same.
> >
>
On Wed, Sep 29, 2021 at 9:07 AM houzj.f...@fujitsu.com
wrote:
>
> On Tues, Sep 28, 2021 10:46 PM vignesh C wrote:
> > Attached v34 patch has the changes for the same.
>
> Thanks for updating the patch.
> Here are a few comments.
>
> 1)
> + * ALL TABLES IN SCHEMA schema [[, ...]
>
>
On Wed, Sep 29, 2021 at 5:48 PM houzj.f...@fujitsu.com
wrote:
>
> On Wed, Sep 29, 2021 5:14 PM Amit Kapila wrote:
> > On Wed, Sep 29, 2021 at 11:59 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Wed, Sep 29, 2021 12:34 PM Amit Kapila
> > > > On Wed, Sep 29, 2021 at 9:07 AM
On Wed, Sep 29, 2021 5:14 PM Amit Kapila wrote:
> On Wed, Sep 29, 2021 at 11:59 AM houzj.f...@fujitsu.com
> wrote:
> >
> > On Wed, Sep 29, 2021 12:34 PM Amit Kapila
> > > On Wed, Sep 29, 2021 at 9:07 AM houzj.f...@fujitsu.com
> > > wrote:
> > > >
> > > > On Tues, Sep 28, 2021 10:46 PM vignesh
On Wed, Sep 29, 2021 at 11:59 AM houzj.f...@fujitsu.com
wrote:
>
> On Wed, Sep 29, 2021 12:34 PM Amit Kapila
> > On Wed, Sep 29, 2021 at 9:07 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Tues, Sep 28, 2021 10:46 PM vignesh C wrote:
> > > > Attached v34 patch has the changes for the
On Wed, Sep 29, 2021 12:34 PM Amit Kapila
> On Wed, Sep 29, 2021 at 9:07 AM houzj.f...@fujitsu.com
> wrote:
> >
> > On Tues, Sep 28, 2021 10:46 PM vignesh C wrote:
> > > Attached v34 patch has the changes for the same.
> >
> > 3)
> > + /*
> > +* Check if setting the relation to a
On Wed, Sep 29, 2021 at 3:16 PM Amit Kapila wrote:
>
> 4.
> + /*
> + * Check if setting the relation to a different schema will result in the
> + * publication having schema and same schema's table in the publication.
> + */
> + if (stmt->objectType == OBJECT_TABLE)
> + {
> + ListCell *lc;
> +
On Tue, Sep 28, 2021 at 8:15 PM vignesh C wrote:
>
> On Mon, Sep 27, 2021 at 12:15 PM houzj.f...@fujitsu.com
> wrote:
>
> Attached v34 patch has the changes for the same.
>
Few comments on v34-0001-Added-schema-level-suppo
On Wed, Sep 29, 2021 at 9:07 AM houzj.f...@fujitsu.com
wrote:
>
> On Tues, Sep 28, 2021 10:46 PM vignesh C wrote:
> > Attached v34 patch has the changes for the same.
>
> 3)
> + /*
> +* Check if setting the relation to a different schema will result in
> the
> +*
On Tues, Sep 28, 2021 10:46 PM vignesh C wrote:
> Attached v34 patch has the changes for the same.
Thanks for updating the patch.
Here are a few comments.
1)
+ * ALL TABLES IN SCHEMA schema [[, ...]
[[ -> [
2)
+ /* ALTER PUBLICATION ... ADD/DROP TABLE/ALL TABLES IN SCHEMA
On Monday, Tuesday, September 28, 2021 10:49 PM, vignesh C
wrote:
>
> Yes this should not be supported, we should throw an error in this case.
> This is handled in the v34 patch attached at [1].
> [1] - https://www.postgresql.org/message-
>
On Tue, Sep 28, 2021 at 4:35 PM tanghy.f...@fujitsu.com
wrote:
>
> On Monday, September 27, 2021 1:32 PM, vignesh C wrote:
>
> >Attached v33 patch has the preprocess_pubobj_list review comment fix
> >suggested by Alvaro at [1]. The
> >v33-0006-Alternate-grammar-for-ALL-TABLES-IN-SCHEMA.patch
On Mon, Sep 27, 2021 at 4:51 PM Greg Nancarrow wrote:
>
> On Mon, Sep 27, 2021 at 2:32 PM vignesh C wrote:
> >
> > Attached v33 patch has the preprocess_pubobj_list review comment fix
> > suggested by Alvaro at [1].
>
> In the v33-0003 patch, there's a couple of error-case tests that have
>
On Mon, Sep 27, 2021 at 2:46 PM Greg Nancarrow wrote:
>
> On Mon, Sep 27, 2021 at 2:32 PM vignesh C wrote:
> >
> > Attached v33 patch has the preprocess_pubobj_list review comment fix
> > suggested by Alvaro at [1].
>
> A minor point I noticed in the v33-0002 patch, in the code added to
> the
On Monday, September 27, 2021 1:32 PM, vignesh C wrote:
>Attached v33 patch has the preprocess_pubobj_list review comment fix
>suggested by Alvaro at [1]. The
>v33-0006-Alternate-grammar-for-ALL-TABLES-IN-SCHEMA.patch patch has
>the grammar changes as suggested by Alvaro at [1]. If we agree this
On Mon, Sep 27, 2021 at 2:32 PM vignesh C wrote:
>
> Attached v33 patch has the preprocess_pubobj_list review comment fix
> suggested by Alvaro at [1].
In the v33-0003 patch, there's a couple of error-case tests that have
comments copied from success-case tests:
+-- should be able to add table
1 - 100 of 322 matches
Mail list logo
