Horton

Re: 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
Cc: Andres Freund <andres@anarazel.de>
Date: 2020-07-19T02:09:41Z
Lists: pgsql-hackers 
I wrote:
> In all branches back to v10, initdb marks pg_subscription.subslotname
> as NOT NULL: ...
> Nonetheless, CREATE/ALTER SUBSCRIPTION blithely set it to null
> when slot_name = NONE is specified.

> 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.

After further thought, it seems like changing the definition that
subslotname is null for "NONE" is unworkable, because client-side
code might be depending on that.  (pg_dump certainly is; we could
change that, but other code might have the same expectation.)

What I propose we do is

(1) Fix the NOT NULL marking in HEAD and v13.  We could perhaps
alter it in older branches as well, but we cannot force initdb
so such a change would only affect newly-initdb'd installations.

(2) In pre-v13 branches, hack the JIT tuple deconstruction code
to be specifically aware that it can't trust attnotnull for
pg_subscription.subslotname.  Yeah, it's ugly, but at least it's
not ugly going forwards.

I haven't looked to see where or how we might do (2), but I assume
it's possible.

> 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.

It seems like at least in assert-enabled builds, we'd better have
a cross-check for that.  I'm not sure where's the best place.

			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.