Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error
Date
Msg-id CAD21AoBzQP=Ss6dVtDrBUWG+iVmdoQB=jWnycd1ALesW+hpMoQ@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Responses Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error  (Kuntal Ghosh <kuntalghosh.2007@gmail.com>)
List pgsql-hackers
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

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: [HACKERS] Re: Create subscription with `create_slot=false` andincorrect slot name
Next
From: Daniel Gustafsson
Date:
Subject: Re: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256