Re: [HACKERS] "create publication..all tables" ignore 'partition notsupported' error

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