From: | Keith Fiske <keith(dot)fiske(at)crunchydata(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-bugs(at)lists(dot)postgresql(dot)org |
Subject: | Re: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist |
Date: | 2019-06-21 17:06:19 |
Message-ID: | CAODZiv6zpj3JnRsC2cM9zcKTKM8yPwM_q05D3ht2YSr+UK38BA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
On Fri, Jun 21, 2019 at 12:12 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Keith Fiske <keith(dot)fiske(at)crunchydata(dot)com> writes:
> > On Thu, Jun 20, 2019 at 9:54 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> Here's a patch against HEAD --- since I'm feeling more mortal than usual
> >> right now, I'll put this out for review rather than just pushing it.
>
> > Can't really provide a thorough code review, but I did apply the patch to
> > the base 11.4 code (not HEAD from github) and the compound ALTER table
> > statement that was failing before now works without error. Thank you for
> > the quick fix!
>
> Thanks for testing! However, I had a nagging feeling that I was still
> missing something, and this morning I realized what. The proposed
> patch basically changes ATExecAlterColumnType's assumptions from
> "no constraint index will have any direct dependencies on table columns"
> to "if a constraint index has a direct dependency on a table column,
> so will its constraint". This is easily shown to not be the case:
>
> regression=# create table foo (f1 int, f2 int);
> CREATE TABLE
> regression=# alter table foo add exclude using btree (f1 with =) where (f2
> > 0);
> ALTER TABLE
> regression=# select pg_describe_object(classid,objid,objsubid) as obj,
> pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype from
> pg_depend where objid >= 'foo'::regclass or refobjid >= 'foo'::regclass;
> obj | ref
> | deptype
>
> -------------------------------------+-------------------------------------+---------
> type foo | table foo
> | i
> type foo[] | type foo
> | i
> table foo | schema public
> | n
> constraint foo_f1_excl on table foo | column f1 of table foo
> | a
> index foo_f1_excl | constraint foo_f1_excl on table foo
> | i
> index foo_f1_excl | column f2 of table foo
> | a
> (6 rows)
>
> Notice that the index has a dependency on column f2 but the constraint
> doesn't. So if we change (just) f2, ATExecAlterColumnType never notices
> the constraint at all, and kaboom:
>
> regression=# alter table foo alter column f2 type bigint;
> ERROR: cannot drop index foo_f1_excl because constraint foo_f1_excl on
> table foo requires it
> HINT: You can drop constraint foo_f1_excl on table foo instead.
>
> This is the same with or without yesterday's patch, and while I didn't
> trouble to verify it, I'm quite sure pre-e76de8861 would fail the same.
>
> I'm not exactly convinced that this dependency structure is a Good Thing,
> but in any case we don't get to rethink it in released branches. So
> we need to make ATExecAlterColumnType cope, and the way to do that seems
> to be to do the get_index_constraint check in that function not later on.
>
> In principle this might lead to a few more duplicative
> get_index_constraint calls than before, because if a constraint index has
> multiple column dependencies we'll have to repeat get_index_constraint for
> each one. But I hardly think that case is worth stressing about the
> performance of, given it never worked at all before this month.
>
> As before, I attach a patch against HEAD, plus one that assumes e76de8861
> has been reverted first, which is likely easier to review.
>
> Unlike yesterday, I'm feeling pretty good about this patch now, but it
> still wouldn't hurt for somebody else to go over it.
>
> regards, tom lane
>
>
Tested applying the patch against HEAD this time. Combined ALTER TABLE
again works without issue.
--
Keith Fiske
Senior Database Engineer
Crunchy Data - http://crunchydata.com
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-06-21 17:27:47 | Re: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist |
Previous Message | Tom Lane | 2019-06-21 16:47:55 | Re: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-06-21 17:27:47 | Re: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist |
Previous Message | Alexander Korotkov | 2019-06-21 17:04:31 | Re: SQL/JSON path issues/questions |