Horton

pg_subscription.subslotname is wrongly marked NOT NULL

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2020-07-18T18:15:39Z
Lists: pgsql-hackers 
In all branches back to v10, initdb marks pg_subscription.subslotname
as NOT NULL:

# \d pg_subscription
             Table "pg_catalog.pg_subscription"
     Column      |  Type   | Collation | Nullable | Default 
-----------------+---------+-----------+----------+---------
 oid             | oid     |           | not null | 
 subdbid         | oid     |           | not null | 
 subname         | name    |           | not null | 
 subowner        | oid     |           | not null | 
 subenabled      | boolean |           | not null | 
 subbinary       | boolean |           | not null | 
 subconninfo     | text    | C         | not null | 
 subslotname     | name    |           | not null | 
 subsynccommit   | text    | C         | not null | 
 subpublications | text[]  | C         | not null | 

Nonetheless, CREATE/ALTER SUBSCRIPTION blithely set it to null
when slot_name = NONE is specified.

This apparently causes few ill effects, unless somebody decides
to JIT-compile deconstruction of pg_subscription tuples.  Which
is why all of Andres' JIT-enabled buildfarm animals are unhappy
with 9de77b545 --- quite unintentionally, that commit added a
test case that exposed the problem.

What would we like to do about this?  Removing the NOT NULL
marking wouldn't be too hard in HEAD, but telling users to
fix it manually in the back branches seems like a mess.

On the whole it seems like changing the code to use some other
representation of slot_name = NONE, like say an empty string,
would be less of a mess.

It's also a bit annoying that we have no mechanized checks that
would catch this inconsistency.  If JIT is going to be absolutely
dependent on NOT NULL markings being accurate, we can't really
have such a laissez-faire attitude to C code getting it wrong.

Thoughts?

			regards, tom lane

Commits

  1. Assert that we don't insert nulls into attnotnull catalog columns.

  2. Avoid direct C access to possibly-null pg_subscription_rel.srsublsn.

  3. Kluge slot_compile_deform() to ignore incorrect attnotnull markings.

  4. Correctly mark pg_subscription_rel.srsublsn as nullable.

  5. Correctly mark pg_subscription.subslotname as nullable.