Обсуждение: set relispartition when attaching child index
Hi, It seems that DefineIndex() is forgetting to update_relispartition() on a partition's index when it's attached to an index being added to the parent. That results in unexpected behavior when adding a foreign key referencing the parent. create table foo (a int) partition by list (a); create table foo1 partition of foo for values in (1); alter table foo1 add primary key (a); alter table foo add primary key (a); select relname, relispartition from pg_class where relname = 'foo1_pkey'; relname | relispartition -----------+---------------- foo1_pkey | f (1 row) create table bar (a int references foo); ERROR: index for 24683 not found in partition foo1 Attached patch fixes that, but I haven't added any new tests. PS: Came to know that that's the case when reading this blog on the new foreign key feature: https://www.depesz.com/2019/04/24/waiting-for-postgresql-12-support-foreign-keys-that-reference-partitioned-tables/ Thanks, Amit
Вложения
On 2019-Apr-25, Amit Langote wrote: > It seems that DefineIndex() is forgetting to update_relispartition() > on a partition's index when it's attached to an index being added to > the parent. That results in unexpected behavior when adding a foreign > key referencing the parent. Ah, thanks for fixing. I also read Depesz's post this morning and was to see what was going on after I push the pg_dump fix. I'll get this pushed later. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-Apr-25, Amit Langote wrote: > It seems that DefineIndex() is forgetting to update_relispartition() > on a partition's index when it's attached to an index being added to > the parent. That results in unexpected behavior when adding a foreign > key referencing the parent. BTW, maybe IndexSetParentIndex ought to be the one calling update_relispartition() ... -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Apr 25, 2019 at 12:35 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > On 2019-Apr-25, Amit Langote wrote: > > > It seems that DefineIndex() is forgetting to update_relispartition() > > on a partition's index when it's attached to an index being added to > > the parent. That results in unexpected behavior when adding a foreign > > key referencing the parent. > > BTW, maybe IndexSetParentIndex ought to be the one calling > update_relispartition() ... I thought so too, but other sites are doing what I did in the patch. Thanks, Amit
On Thu, Apr 25, 2019 at 12:38 AM Amit Langote <amitlangote09@gmail.com> wrote: > On Thu, Apr 25, 2019 at 12:35 AM Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: > > On 2019-Apr-25, Amit Langote wrote: > > > > > It seems that DefineIndex() is forgetting to update_relispartition() > > > on a partition's index when it's attached to an index being added to > > > the parent. That results in unexpected behavior when adding a foreign > > > key referencing the parent. > > > > BTW, maybe IndexSetParentIndex ought to be the one calling > > update_relispartition() ... > > I thought so too, but other sites are doing what I did in the patch. Although, we wouldn't have this bug if it was IndexSetParentIndex calling it. Maybe a good idea to do that now. Thanks, Amit
On Thu, Apr 25, 2019 at 12:39 AM Amit Langote <amitlangote09@gmail.com> wrote: > On Thu, Apr 25, 2019 at 12:38 AM Amit Langote <amitlangote09@gmail.com> wrote: > > On Thu, Apr 25, 2019 at 12:35 AM Alvaro Herrera > > <alvherre@2ndquadrant.com> wrote: > > > On 2019-Apr-25, Amit Langote wrote: > > > > > > > It seems that DefineIndex() is forgetting to update_relispartition() > > > > on a partition's index when it's attached to an index being added to > > > > the parent. That results in unexpected behavior when adding a foreign > > > > key referencing the parent. > > > > > > BTW, maybe IndexSetParentIndex ought to be the one calling > > > update_relispartition() ... > > > > I thought so too, but other sites are doing what I did in the patch. > > Although, we wouldn't have this bug if it was IndexSetParentIndex > calling it. Maybe a good idea to do that now. I tried that in the attached. Thanks, Amit
Вложения
On 2019/04/25 0:55, Amit Langote wrote: > On Thu, Apr 25, 2019 at 12:39 AM Amit Langote <amitlangote09@gmail.com> wrote: >> On Thu, Apr 25, 2019 at 12:38 AM Amit Langote <amitlangote09@gmail.com> wrote: >>> On Thu, Apr 25, 2019 at 12:35 AM Alvaro Herrera >>> <alvherre@2ndquadrant.com> wrote: >>>> On 2019-Apr-25, Amit Langote wrote: >>>> >>>>> It seems that DefineIndex() is forgetting to update_relispartition() >>>>> on a partition's index when it's attached to an index being added to >>>>> the parent. That results in unexpected behavior when adding a foreign >>>>> key referencing the parent. >>>> >>>> BTW, maybe IndexSetParentIndex ought to be the one calling >>>> update_relispartition() ... >>> >>> I thought so too, but other sites are doing what I did in the patch. >> >> Although, we wouldn't have this bug if it was IndexSetParentIndex >> calling it. Maybe a good idea to do that now. > > I tried that in the attached. BTW, this will need to be back-patched to 11. Thanks, Amit
On 2019-Apr-25, Amit Langote wrote: > BTW, this will need to be back-patched to 11. Done, thanks for the patch. I added the test in master, but obviously it doesn't work in pg11, so I just verified manually that relispartition is set correctly. I don't think it's worth doing more, though there are other things that are affected by a bogus relispartition marking for an index (example: creating the index in the last partition that didn't have it, should mark the index on parent valid; I think that would fail to propagate to upper levels correctly.) -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019/04/26 23:12, Alvaro Herrera wrote: > On 2019-Apr-25, Amit Langote wrote: > >> BTW, this will need to be back-patched to 11. > > Done, thanks for the patch. I added the test in master, but obviously > it doesn't work in pg11, so I just verified manually that relispartition > is set correctly. Thank you. > I don't think it's worth doing more, though there are > other things that are affected by a bogus relispartition marking for an > index (example: creating the index in the last partition that didn't > have it, should mark the index on parent valid; I think that would fail > to propagate to upper levels correctly.) Hmm, I couldn't see any misbehavior for this example: create table p (a int, b int) partition by list (a); create table p1 partition of p for values in (1) partition by list (b); create table p11 partition of p1 for values in (1); create index on only p (a); create index on only p1 (a); alter index p_a_idx attach partition p1_a_idx ; select relname, relispartition from pg_class where relname like 'p%idx'; relname │ relispartition ──────────┼──────────────── p_a_idx │ f p1_a_idx │ t (2 rows) \d p Table "public.p" Column │ Type │ Collation │ Nullable │ Default ────────┼─────────┼───────────┼──────────┼───────── a │ integer │ │ │ b │ integer │ │ │ Partition key: LIST (a) Indexes: "p_a_idx" btree (a) INVALID Number of partitions: 1 (Use \d+ to list them.) create index on p11 (a); alter index p1_a_idx attach partition p11_a_idx ; select relname, relispartition from pg_class where relname like 'p%idx'; relname │ relispartition ───────────┼──────────────── p_a_idx │ f p1_a_idx │ t p11_a_idx │ t (3 rows) \d p Table "public.p" Column │ Type │ Collation │ Nullable │ Default ────────┼─────────┼───────────┼──────────┼───────── a │ integer │ │ │ b │ integer │ │ │ Partition key: LIST (a) Indexes: "p_a_idx" btree (a) Number of partitions: 1 (Use \d+ to list them.) Maybe, because the code path we fixed has nothing to do with this case? Thanks, Amit