set relispartition when attaching child index

Started by Amit Langotealmost 7 years ago9 messageshackers
Jump to latest
#1Amit Langote
Langote_Amit_f8@lab.ntt.co.jp

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

Attachments:

DefineIndex-update_relispartition.patchapplication/octet-stream; name=DefineIndex-update_relispartition.patchDownload+5-3
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Langote (#1)
Re: set relispartition when attaching child index

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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Langote (#1)
Re: set relispartition when attaching child index

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

#4Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Alvaro Herrera (#3)
Re: set relispartition when attaching child index

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

#5Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#4)
Re: set relispartition when attaching child index

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

#6Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#5)
Re: set relispartition when attaching child index

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

Attachments:

IndexSetParentIndex-update_relispartition.patchapplication/octet-stream; name=IndexSetParentIndex-update_relispartition.patchDownload+32-37
#7Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#6)
Re: set relispartition when attaching child index

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

#8Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Langote (#7)
Re: set relispartition when attaching child index

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

#9Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Alvaro Herrera (#8)
Re: set relispartition when attaching child index

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