ALTER TABLE results in "ERROR: could not open relation with OID 43707388"

On Thu, Jul 4, 2019 at 7:42 AM Manuel Rigger <rigger.manuel@gmail.com>
wrote:

Hi everyone,

I would expect that the statements are equivalent to the following,
where the ALTER TABLE actions were split up, and which execute without
errors:

I'm using the following Postgres version: psql (11.4 (Ubuntu
11.4-1.pgdg19.04+1)).

This has been fixed for version 11.5 which is as yet unreleased.

David J.

Hi David,

Thanks for the quick response! Do you know which commit fixed it?

Best,
Manuel

On Thu, Jul 4, 2019 at 6:24 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:

On Thu, Jul 4, 2019 at 9:41 AM Manuel Rigger <rigger.manuel@gmail.com>
wrote:

Thanks for the quick response! Do you know which commit fixed it?

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=da1041fc3a2b65a6a36f1b8b91765a46e54e571e

David J.

Thanks again!

Best,
Manuel

On Thu, Jul 4, 2019 at 7:07 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Thu, Jul 4, 2019 at 7:42 AM Manuel Rigger <rigger.manuel@gmail.com>
wrote:

I would expect that the statements are equivalent to the following,
where the ALTER TABLE actions were split up, and which execute without
errors:

This has been fixed for version 11.5 which is as yet unreleased.

Really? Cause it still fails in HEAD for me. It might be related to
that new-in-11.4 bug, or to the old bug we were trying to fix, but
I fear it's a distinct problem. I've not had time to dig into the code
though.

regards, tom lane

On Thu, Jul 04, 2019 at 06:50:54PM -0400, Tom Lane wrote:

Really? Cause it still fails in HEAD for me. It might be related to
that new-in-11.4 bug, or to the old bug we were trying to fix, but
I fear it's a distinct problem. I've not had time to dig into the code
though.

This is failing on HEAD (d5ab9df), 11.3 and 11.4. So that looks like
a separate, not fixed, issue to me.
--
Michael

On Thursday, July 4, 2019, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Thu, Jul 4, 2019 at 7:42 AM Manuel Rigger <rigger.manuel@gmail.com>
wrote:

I would expect that the statements are equivalent to the following,
where the ALTER TABLE actions were split up, and which execute without
errors:

This has been fixed for version 11.5 which is as yet unreleased.

Really? Cause it still fails in HEAD for me. It might be related to
that new-in-11.4 bug, or to the old bug we were trying to fix, but
I fear it's a distinct problem. I've not had time to dig into the code
though.

Yeah, it looked to be the same/similar behavior to me but I didn’t try and
confirm that it was indeed fixed by that patch.

David J.

Manuel Rigger <rigger.manuel@gmail.com> writes:

CREATE TABLE t0(c0 INT);
CREATE UNIQUE INDEX i0 ON t0(c0);
ALTER TABLE t0 ADD PRIMARY KEY USING INDEX i0, ALTER c0 TYPE BIGINT;
-- unexpected: ERROR: could not open relation with OID 43707388

The sequence of events here is that:

1. transformIndexConstraint looks up the index "i0" and saves its OID
in the IndexStmt's indexOid field.

2. ALTER c0 TYPE BIGINT executes first, because of ALTER TABLE's
pass design. It rebuilds the i0 index --- with a new OID.

3. ATExecAddIndexConstraint tries to look up i0 using the old OID.
Kaboom.

Really, it's a horrible idea that parse analysis of the ALTER
commmand is looking up the index at all; that should be postponed
until execution. I tried to refactor things so that we did it
that way, but it turns out that there's an additional side-effect
that happens at parse analysis: if the constraint is PRIMARY KEY
not just UNIQUE, we add implicit SET NOT NULL subcommands to make
sure all the columns are NOT NULL. That's what allows this
example to work:

regression=# create table foo (f1 int);
CREATE TABLE
regression=# create unique index fooi on foo (f1);
CREATE INDEX
regression=# alter table foo add primary key using index fooi;
ALTER TABLE
regression=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
f1 | integer | | not null |
Indexes:
"fooi" PRIMARY KEY, btree (f1)

Perhaps we could drop that behavior and insist that you must have
already set up not-nullness of the pkey columns in order to use
ADD PRIMARY KEY USING INDEX. But I bet somebody would complain.

So, in order to fix this properly, we have to postpone the index
lookup into ALTER TABLE execution *and* be willing to generate
SET NOT NULL subcommands on-the-fly during execution.

This seems probably do-able, but it's mighty closely related to
the problems discussed in

/messages/by-id/10365.1558909428@sss.pgh.pa.us

wherein I said

Looking into parse_utilcmd.c with an eye to making it do that, I almost
immediately ran across bugs we hadn't even known were there in ALTER TABLE
ADD/DROP GENERATED. These have got a different but arguably-related
flavor of bug: they are making decisions inside transformAlterTableStmt
that might be wrong by the time we get to execution. ...

So I'm inclined to put this on the back burner until we have some
consensus how to proceed on that.

It seems likely to me that the cleanest fix, for both this issue
and the ADD/DROP GENERATED ones, is to add a new ALTER TABLE pass
that runs after AT_PASS_ALTER_TYPE and AT_PASS_ADD_COL and does
parse analysis activities for subcommands that could depend on the
results of those steps. The parse analysis would result in adding
new subcommands into the queues for AT_PASS_COL_ATTRS,
AT_PASS_ADD_CONSTR, and maybe other late-stage passes. Needing
to run parse analysis activities at this phase is another reason
for extending AlterTable's API as I proposed in that thread ---
in particular, we really want access to the queryString so we
can pass it down to parse analysis for possible use in error messages.

(More generally, I wonder whether we really want initial parse
analysis doing *anything* for ALTER TABLE. Perhaps we ought to
refactor so that we always do that work on-the-fly, one subcommand
at a time.)

regards, tom lane

I wrote:

Manuel Rigger <rigger.manuel@gmail.com> writes:

CREATE TABLE t0(c0 INT);
CREATE UNIQUE INDEX i0 ON t0(c0);
ALTER TABLE t0 ADD PRIMARY KEY USING INDEX i0, ALTER c0 TYPE BIGINT;
-- unexpected: ERROR: could not open relation with OID 43707388

The sequence of events here is that:
1. transformIndexConstraint looks up the index "i0" and saves its OID
in the IndexStmt's indexOid field.
2. ALTER c0 TYPE BIGINT executes first, because of ALTER TABLE's
pass design. It rebuilds the i0 index --- with a new OID.
3. ATExecAddIndexConstraint tries to look up i0 using the old OID.
Kaboom.

FYI, the patch I posted at

/messages/by-id/24573.1572647175@sss.pgh.pa.us

fixes this (as well as your other similar complaint). I forgot to
add a regression test matching this case, but will do so later.

regards, tom lane

Thanks a lot for the patch and for informing me about the fix!

Best,
Manuel