Re: "create publication..all tables" ignore 'partition not supported' error

On Tue, May 30, 2017 at 1:42 PM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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