Re: BUG #18558: ALTER PUBLICATION fails with unhelpful error on attempt to use system column

On Sat, Jul 27, 2024 at 11:15 PM PG Bug reporting form
<noreply(at)postgresql(dot)org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 18558
> Logged by: Alexander Lakhin
> Email address: exclusion(at)gmail(dot)com
> PostgreSQL version: 17beta2
> Operating system: Ubuntu 22.04
> Description:
>
> The following script:
> CREATE TABLE t(a int);
> CREATE PUBLICATION p FOR TABLE t(a);
>
> ALTER PUBLICATION p SET TABLE t (a, ctid);
> triggers
> ERROR: negative bitmapset member not allowed
>
> Whilst:
> CREATE PUBLICATION p FOR TABLE t(a, ctid);
> ends up with a more informative
> ERROR: cannot use system column "ctid" in publication column list
>
> Reproduced on REL_15_STABLE .. master.
>

Thank you for reporting the inconsistent/unfriendly error message.

~~~

The good message::
The good error message comes from publication_translate_columns()
called by publication_add_relation(). This is why the good message
happens for CREATE PUBLICATION. So, you will also find that ALTER
PUBLICATION .. ADD TABLE would give this same good error message:

test_pub=# CREATE TABLE t2(a int);
CREATE TABLE
test_pub=# ALTER PUBLICATION p ADD TABLE t2(a, ctid);
ERROR: cannot use system column "ctid" in publication column list

~~~

The bad message:
OTOH, "ALTER PUBLICATION ... SET TABLE" uses different logic; it first
builds a BitMapSet (BMS) for the old and new column lists to compare
what has changed. No validation is done here before building the new
BMS so it results in the low-level (not user-friendly) error message
caused by the "ctid" column.

~~~

My fix:
I feel the ALTER ... SET and CREATE PUBLICATION the same column list
validation logic. But instead of cut/pasting that validation checking
from publication_translate_columns(), attached is a diff patch that
exposes the publication_translate_columns() so we can just call that
same function.

Result:
Now all these scenarios will produce the same good error, below.

test_pub=# CREATE TABLE t(a int);
CREATE TABLE

test_pub=# CREATE PUBLICATION p FOR TABLE t(a, cid);
2024-07-29 11:30:16.809 AEST [2281] ERROR: column "cid" of relation
"t" does not exist
2024-07-29 11:30:16.809 AEST [2281] STATEMENT: CREATE PUBLICATION p
FOR TABLE t(a, cid);
ERROR: column "cid" of relation "t" does not exist

test_pub=# CREATE PUBLICATION p FOR TABLE t(a);
CREATE PUBLICATION

test_pub=# ALTER PUBLICATION p SET TABLE t(a, ctid);
2024-07-29 11:30:36.579 AEST [2281] ERROR: cannot use system column
"ctid" in publication column list
2024-07-29 11:30:36.579 AEST [2281] STATEMENT: ALTER PUBLICATION p
SET TABLE t(a, ctid);
ERROR: cannot use system column "ctid" in publication column list

~~~

If this is deemed an acceptable fix, then I will improve on it (e.g.
IMO publication_translate_columns can modified to return the BMS), and
I will also add the necessary test cases.

======
Kind Regards,
Peter Smith.
Fujitsu Australia.