Andres Freund <andres@anarazel.de> writes:
> On 2019-04-22 12:33:24 -0400, Tom Lane wrote:
>> But TBH, now that I look at the code, I think the entire optimization
>> is a bad idea and should be removed. Am I right in thinking that the
>> presence of a wrong attnotnull marker could cause the generated code to
>> actually crash, thanks to not checking the tuple's natts field? I don't
>> have enough faith in our enforcement of those constraints to want to see
>> JIT taking that risk to save a nanosecond or two.
> It's not a minor optimization, it's very measurable.
Doesn't matter, if it's unsafe.
>> (Possibly I'd not think this if I weren't fresh off a couple of days
>> with my nose in the ALTER TABLE SET NOT NULL code. But right now,
>> I think that believing that that code does not and never will have
>> any bugs is just damfool.)
> But there's plenty places where we rely on NOT NULL actually working?
I do not think there are any other places where we make this particular
assumption. Given the number of ways in which we rely on there being
natts checks to avoid rewriting tables, I'm very afraid of the idea
that JIT is making more assumptions than the mainline code does.
In hopes of putting some fear into you too, I exhibit the following
behavior, which is not a bug according to our current definitions:
regression=# create table pp(f1 int);
CREATE TABLE
regression=# create table cc() inherits (pp);
CREATE TABLE
regression=# insert into cc values(1);
INSERT 0 1
regression=# insert into cc values(2);
INSERT 0 1
regression=# insert into cc values(null);
INSERT 0 1
regression=# alter table pp add column f2 text;
ALTER TABLE
regression=# alter table pp add column f3 text;
ALTER TABLE
regression=# alter table only pp alter f3 set not null;
ALTER TABLE
regression=# select * from pp;
f1 | f2 | f3
----+----+----
1 | |
2 | |
| |
(3 rows)
The tuples coming out of cc will still have natts = 1, I believe.
If they were deformed according to pp's tupdesc, there'd be a
problem. Now, we shouldn't do that, because this is not the only
possible discrepancy between parent and child tupdescs --- but
I think this example shows that attnotnull is a lot spongier
than you are assuming, even without considering the possibility
of outright bugs.
regards, tom lane