Thread: [HACKERS] "create publication..all tables" ignore 'partition not supported'error
Hi, I observed that - "create publication..all tables" ignore 'partition not supported' error \\create a partition table You are now connected to database "s" as user "centos". s=# CREATE TABLE measurement ( s(# city_id int not null, s(# logdate date not null, s(# peaktemp int, s(# unitsales int s(# ) PARTITION BY RANGE (logdate); CREATE TABLE s=# \\try to publish only this table s=# create publication p for table measurement; ERROR: "measurement" is a partitioned table DETAIL: Adding partitioned tables to publications is not supported. HINT: You can add the table partitions individually. \\try to create publication for all tables s=# create publication p for all tables ; CREATE PUBLICATION s=# \d+ measurement Table "public.measurement" Column | Type | Collation | Nullable| Default | Storage | Stats target | Description -----------+---------+-----------+----------+---------+---------+--------------+------------- city_id | integer | | not null | | plain | | logdate | date | | not null | | plain | | peaktemp | integer | | | | plain | | unitsales | integer | | | | plain | | Partition key: RANGE (logdate) Publications: "p" Publication 'p' has been set against partition table ,which is not supported. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Kuntal Ghosh
Date:
Yeah, it's a bug. While showing the table definition, we use the following query for showing the related publications: "SELECT pub.pubname\n" " FROM pg_catalog.pg_publication pub\n" "LEFT JOIN pg_catalog.pg_publication_rel pr\n" " ON (pr.prpubid = pub.oid)\n" "WHERE pr.prrelid = '%s' OR pub.puballtables\n" "ORDER BY 1;" When pub.puballtables is TRUE, we should also check whether the relation is publishable or not.(Something like is_publishable_class method in pg_publication.c). However, I'm not sure whether this is the correct way to solve the problem. On Mon, May 22, 2017 at 2:39 PM, tushar <tushar.ahuja@enterprisedb.com> wrote: > Hi, > > I observed that - "create publication..all tables" ignore 'partition not > supported' error > > \\create a partition table > > You are now connected to database "s" as user "centos". > s=# CREATE TABLE measurement ( > s(# city_id int not null, > s(# logdate date not null, > s(# peaktemp int, > s(# unitsales int > s(# ) PARTITION BY RANGE (logdate); > CREATE TABLE > s=# > > \\try to publish only this table > > s=# create publication p for table measurement; > ERROR: "measurement" is a partitioned table > DETAIL: Adding partitioned tables to publications is not supported. > HINT: You can add the table partitions individually. > > \\try to create publication for all tables > s=# create publication p for all tables ; > CREATE PUBLICATION > s=# \d+ measurement > Table "public.measurement" > Column | Type | Collation | Nullable | Default | Storage | Stats > target | Description > -----------+---------+-----------+----------+---------+---------+--------------+------------- > city_id | integer | | not null | | plain | > | > logdate | date | | not null | | plain | > | > peaktemp | integer | | | | plain | > | > unitsales | integer | | | | plain | > | > Partition key: RANGE (logdate) > Publications: > "p" > > Publication 'p' has been set against partition table ,which is not > supported. > > -- > regards,tushar > EnterpriseDB https://www.enterprisedb.com/ > The Enterprise PostgreSQL Company > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Amit Langote
Date:
On 2017/05/22 20:02, Kuntal Ghosh wrote: > Yeah, it's a bug. While showing the table definition, we use the > following query for showing the related publications: > "SELECT pub.pubname\n" > " FROM pg_catalog.pg_publication pub\n" > " LEFT JOIN pg_catalog.pg_publication_rel pr\n" > " ON (pr.prpubid = pub.oid)\n" > "WHERE pr.prrelid = '%s' OR pub.puballtables\n" > "ORDER BY 1;" > > When pub.puballtables is TRUE, we should also check whether the > relation is publishable or not.(Something like is_publishable_class > method in pg_publication.c). BTW, you can see the following too, because of the quoted query: create publication pub2 for all tables; -- system tables are not publishable, but... \d pg_class <snip> Publications: "pub2" -- unlogged tables are not publishable, but... create unlogged table foo (a int); \d foo <snip> Publications: "pub2" -- temp tables are not publishable, but... create temp table bar (a int); \d bar <snip> Publications: "pub2" The above contradicts what check_publication_add_tables() thinks are publishable relations. At first, I thought this would be fixed by simply adding a check on relkind and relpersistence in the above query, both of which are available to describeOneTableDetails() already. But, it won't be possible to check the system table status using the available information, so we might need to add a SQL-callable version of is_publishable_class() after all. > > However, I'm not sure whether this is the correct way to solve the problem. AFAICS, the problem is with psql's describe query itself and no other parts of the system are affected by this. So, fixing the query after adding appropriate backend support will do, I think. Thanks, Amit
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Masahiko Sawada
Date:
On Tue, May 30, 2017 at 1:42 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > On 2017/05/22 20:02, Kuntal Ghosh wrote: >> Yeah, it's a bug. While showing the table definition, we use the >> following query for showing the related publications: >> "SELECT pub.pubname\n" >> " FROM pg_catalog.pg_publication pub\n" >> " LEFT JOIN pg_catalog.pg_publication_rel pr\n" >> " ON (pr.prpubid = pub.oid)\n" >> "WHERE pr.prrelid = '%s' OR pub.puballtables\n" >> "ORDER BY 1;" >> >> When pub.puballtables is TRUE, we should also check whether the >> relation is publishable or not.(Something like is_publishable_class >> method in pg_publication.c). > > BTW, you can see the following too, because of the quoted query: > > create publication pub2 for all tables; > > -- system tables are not publishable, but... > \d pg_class > <snip> > Publications: > "pub2" > > -- unlogged tables are not publishable, but... > create unlogged table foo (a int); > \d foo > <snip> > Publications: > "pub2" > > -- temp tables are not publishable, but... > create temp table bar (a int); > \d bar > <snip> > Publications: > "pub2" > > The above contradicts what check_publication_add_tables() thinks are > publishable relations. > > At first, I thought this would be fixed by simply adding a check on > relkind and relpersistence in the above query, both of which are available > to describeOneTableDetails() already. But, it won't be possible to check > the system table status using the available information, so we might need > to add a SQL-callable version of is_publishable_class() after all. I'd say we can fix this issue by just changing the query. Attached patch changes the query so that it can handle publication name correctly, the query gets complex, though. >> >> However, I'm not sure whether this is the correct way to solve the problem. > > AFAICS, the problem is with psql's describe query itself and no other > parts of the system are affected by this. I think so too. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Kuntal Ghosh
Date:
On Wed, May 31, 2017 at 12:58 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > I'd say we can fix this issue by just changing the query. Attached > patch changes the query so that it can handle publication name > correctly, the query gets complex, though. > In is_publishable_class function, there are four conditions to decide whether this is a publishable class or not. 1. relkind == RELKIND_RELATION 2. IsCatalogClass() 3. relpersistence == 'p' 4. relid >= FirstNormalObjectId /* Skip tables created during initdb */ I think the modified query should have a check for the fourth condition as well. Added to open items. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Peter Eisentraut
Date:
On 5/31/17 02:17, Kuntal Ghosh wrote: > On Wed, May 31, 2017 at 12:58 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: >> >> I'd say we can fix this issue by just changing the query. Attached >> patch changes the query so that it can handle publication name >> correctly, the query gets complex, though. >> > In is_publishable_class function, there are four conditions to decide > whether this is a publishable class or not. > > 1. relkind == RELKIND_RELATION > 2. IsCatalogClass() > 3. relpersistence == 'p' > 4. relid >= FirstNormalObjectId /* Skip tables created during initdb */ > > I think the modified query should have a check for the fourth condition as well. The query should be fixed like in the attached patch. pg_get_publication_tables() ends up calling is_publishable_class() internally. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Amit Langote
Date:
On 2017/06/01 10:26, Peter Eisentraut wrote: > On 5/31/17 02:17, Kuntal Ghosh wrote: >> On Wed, May 31, 2017 at 12:58 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: >>> >>> I'd say we can fix this issue by just changing the query. Attached >>> patch changes the query so that it can handle publication name >>> correctly, the query gets complex, though. >>> >> In is_publishable_class function, there are four conditions to decide >> whether this is a publishable class or not. >> >> 1. relkind == RELKIND_RELATION >> 2. IsCatalogClass() >> 3. relpersistence == 'p' >> 4. relid >= FirstNormalObjectId /* Skip tables created during initdb */ >> >> I think the modified query should have a check for the fourth condition as well. > > The query should be fixed like in the attached patch. > pg_get_publication_tables() ends up calling is_publishable_class() > internally. Looks good to me. Thanks, Amit
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Kuntal Ghosh
Date:
On Thu, Jun 1, 2017 at 6:56 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 5/31/17 02:17, Kuntal Ghosh wrote: >> On Wed, May 31, 2017 at 12:58 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: >>> >>> I'd say we can fix this issue by just changing the query. Attached >>> patch changes the query so that it can handle publication name >>> correctly, the query gets complex, though. >>> >> In is_publishable_class function, there are four conditions to decide >> whether this is a publishable class or not. >> >> 1. relkind == RELKIND_RELATION >> 2. IsCatalogClass() >> 3. relpersistence == 'p' >> 4. relid >= FirstNormalObjectId /* Skip tables created during initdb */ >> >> I think the modified query should have a check for the fourth condition as well. > > The query should be fixed like in the attached patch. > pg_get_publication_tables() ends up calling is_publishable_class() > internally. > Works fine. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
From
Peter Eisentraut
Date:
On 5/31/17 21:26, Peter Eisentraut wrote: > On 5/31/17 02:17, Kuntal Ghosh wrote: >> On Wed, May 31, 2017 at 12:58 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: >>> >>> I'd say we can fix this issue by just changing the query. Attached >>> patch changes the query so that it can handle publication name >>> correctly, the query gets complex, though. >>> >> In is_publishable_class function, there are four conditions to decide >> whether this is a publishable class or not. >> >> 1. relkind == RELKIND_RELATION >> 2. IsCatalogClass() >> 3. relpersistence == 'p' >> 4. relid >= FirstNormalObjectId /* Skip tables created during initdb */ >> >> I think the modified query should have a check for the fourth condition as well. > > The query should be fixed like in the attached patch. > pg_get_publication_tables() ends up calling is_publishable_class() > internally. committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services