Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation

On 14.11.21 13:18, Bharath Rupireddy wrote:
> PSA v11 patch with 2 APIs with much simpler parameters and small
> function names:
>
> int errdetail_rel(Form_pg_class rd_rel);
> int errdetail_rel_v2(Oid relid, char relkind, char relpersistence);
>
> Please review it.

I think this is not an improvement. It loses the ability of the caller
the specify exactly why a relation is not acceptable. Before, a caller
could say, it's the wrong relkind, or it's the wrong persistence, or
whatever. Now, it just spits out some details about the relation, but
you can't control which. It could easily be wrong, too: AFAICT, this
will complain that a temporary table is not supported, but it could also
be that a table in general is not supported.

In my mind, this leads us back into the mess that we have before
errdetail_relkind_not_supported(): Very detailed error messages that
didn't actually hit the point.

I think a separate errdetail_relpersistence_not_supported() would be a
better solution (assuming there are enough callers to make it worth a
separate function).