Обсуждение: MERGE PARTITIONS and DEPENDS ON EXTENSION.

Поиск
Список
Период
Сортировка

MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Kirill Reshke
Дата:
Today, while reviewing another patch, I spotted PostgreSQL behaviour
which I cannot tell if is correct.

-- create relation
reshke=# create table pt (i int) partition by range ( i);
CREATE TABLE

-- create partitions.
reshke=# create table pt1 partition of pt for values from ( 1 ) to (2) ;
CREATE TABLE
reshke=# create table pt2 partition of pt for values from ( 2 ) to (3) ;
CREATE TABLE

-- manually add dependency on extension.
reshke=# alter index pt1_i_idx depends on extension btree_gist ;
ALTER INDEX
reshke=# alter index pt2_i_idx depends on extension btree_gist ;
ALTER INDEX

At this point, `drop extension btree_gist` fails due to existing
dependencies. However, after `alter table pt merge partitions ( pt1 ,
pt2 ) into pt3;` there are no dependencies, and drop extension
executes successfully.

My first impression was that there is no issue as the user created a
new database object, so should manually add dependency on extension.
However I am not 100% in this reasoning.

Any thoughts?

-- 
Best regards,
Kirill Reshke



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Matheus Alcantara
Дата:
Hi,

On 10/03/26 14:53, Kirill Reshke wrote:
> Today, while reviewing another patch, I spotted PostgreSQL behaviour
> which I cannot tell if is correct.
> 
> -- create relation
> reshke=# create table pt (i int) partition by range ( i);
> CREATE TABLE
> 
> -- create partitions.
> reshke=# create table pt1 partition of pt for values from ( 1 ) to (2) ;
> CREATE TABLE
> reshke=# create table pt2 partition of pt for values from ( 2 ) to (3) ;
> CREATE TABLE
> 
> -- manually add dependency on extension.
> reshke=# alter index pt1_i_idx depends on extension btree_gist ;
> ALTER INDEX
> reshke=# alter index pt2_i_idx depends on extension btree_gist ;
> ALTER INDEX
> 
> At this point, `drop extension btree_gist` fails due to existing
> dependencies. However, after `alter table pt merge partitions ( pt1 ,
> pt2 ) into pt3;` there are no dependencies, and drop extension
> executes successfully.
> 
> My first impression was that there is no issue as the user created a
> new database object, so should manually add dependency on extension.
> However I am not 100% in this reasoning.
> 
> Any thoughts?
> 

I'm also not sure if it's correct to assume that the dependency should 
be manually added after a partition is merged or splited but I was 
checking ATExecMergePartitions() and ATExecSplitPartition() and I 
think that it's not complicated to implement this.

IIUC we just need to collect the extension dependencies before an 
index is detached on MERGE and SPLIT operations and then apply the 
dependency after the index is created on the new merged/splited 
partition. The attached patch implement this.

Note that I'm using two different extensions for partition_merge and 
partition_split tests because I was having deadlock issues when 
running these tests in parallel using the same extension as a dependency.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com
Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Wed Mar 11, 2026 at 12:12 PM -03, Matheus Alcantara wrote:
> On 10/03/26 14:53, Kirill Reshke wrote:
>> Today, while reviewing another patch, I spotted PostgreSQL behaviour
>> which I cannot tell if is correct.
>>
>> -- create relation
>> reshke=# create table pt (i int) partition by range ( i);
>> CREATE TABLE
>>
>> -- create partitions.
>> reshke=# create table pt1 partition of pt for values from ( 1 ) to (2) ;
>> CREATE TABLE
>> reshke=# create table pt2 partition of pt for values from ( 2 ) to (3) ;
>> CREATE TABLE
>>
>> -- manually add dependency on extension.
>> reshke=# alter index pt1_i_idx depends on extension btree_gist ;
>> ALTER INDEX
>> reshke=# alter index pt2_i_idx depends on extension btree_gist ;
>> ALTER INDEX
>>
>> At this point, `drop extension btree_gist` fails due to existing
>> dependencies. However, after `alter table pt merge partitions ( pt1 ,
>> pt2 ) into pt3;` there are no dependencies, and drop extension
>> executes successfully.
>>
>> My first impression was that there is no issue as the user created a
>> new database object, so should manually add dependency on extension.
>> However I am not 100% in this reasoning.
>>
>> Any thoughts?
>>
>
> I'm also not sure if it's correct to assume that the dependency should
> be manually added after a partition is merged or splited but I was
> checking ATExecMergePartitions() and ATExecSplitPartition() and I
> think that it's not complicated to implement this.
>
> IIUC we just need to collect the extension dependencies before an
> index is detached on MERGE and SPLIT operations and then apply the
> dependency after the index is created on the new merged/splited
> partition. The attached patch implement this.
>
> Note that I'm using two different extensions for partition_merge and
> partition_split tests because I was having deadlock issues when
> running these tests in parallel using the same extension as a dependency.
>

Attaching a new rebased version, no changes compared with v1.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Dmitry Koval
Дата:
Hi Matheus!

Thank you for patch.
I agree that dependency should be automatically added for SPLIT 
PARTITION. But I'm not sure about MERGE PARTITION ...
Might be it would be more correct to automatically add a dependency only 
if all merged partitions have it?

-- 
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Tue Apr 14, 2026 at 6:05 AM -03, Dmitry Koval wrote:
> Hi Matheus!
>
> Thank you for patch.
> I agree that dependency should be automatically added for SPLIT
> PARTITION. But I'm not sure about MERGE PARTITION ...
> Might be it would be more correct to automatically add a dependency only
> if all merged partitions have it?

Hi,

Thank you for taking a look on this!

I agree with your suggestion. The attached patch implements the
intersection behavior for MERGE PARTITIONS: extension dependencies are
only preserved on the merged partition's index if all source partition
indexes have that dependency.

For example:
MERGE(idx1(ext_a, ext_b), idx2(ext_a)) -> idx3(ext_a)  -- only ext_a is common
MERGE(idx1(ext_a), idx2())             -> idx3()       -- no common deps

For SPLIT PARTITION, the behavior remains the same since there's only
one source partition, all its extension dependencies are copied to the
new partition indexes.

While working on this patch, I noticed what might be a separate bug (or
perhaps intentional behavior that I don't understand): extension
dependencies on parent partitioned indexes don't seem to prevent DROP
EXTENSION, but dependencies on child partition indexes do. See this
example:

CREATE EXTENSION IF NOT EXISTS btree_gist;

CREATE TABLE t (i int) PARTITION BY RANGE (i);
CREATE TABLE t_1 PARTITION OF t FOR VALUES FROM (1) TO (2);
CREATE INDEX t_idx ON t USING gist (i);

-- Add dependency on the PARENT partitioned index
ALTER INDEX t_idx DEPENDS ON EXTENSION btree_gist;

-- This succeeds (I expected it to fail):
DROP EXTENSION btree_gist;

But if I add the dependency on the child partition index instead:

ALTER INDEX t_1_i_idx DEPENDS ON EXTENSION btree_gist;

DROP EXTENSION btree_gist;
ERROR:  cannot drop extension btree_gist because other objects depend on it
DETAIL:  index t_idx depends on operator class gist_int4_ops for access method gist

Is this expected behavior, or a separate bug? I would have expected the
dependency on the parent index to also prevent the DROP.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Dmitry Koval
Дата:
Hi Matheus!

 >v3-0001-Preserve-extension-dependencies-on-indexes-during.patch

I looked patch and did not find problems. But there is one point: 
extensions btree_gist, btree_gin, citext are not included by default.
So command

 > ./configure --enable-debug --enable-cassert --prefix `pwd`/install
 > >/dev/null && make -s && make install -s && make check

generates errors like

ERROR:  extension "btree_gist" is not available
ERROR:  extension "btree_gin" is not available
ERROR:  extension "citext" is not available

Might be it would be better to use for tests extensions from the
catalog src/test/modules/test_extensions (see test
src/test/modules/test_extensions/sql/test_extdepend.sql)?

 >extension dependencies on parent partitioned indexes don't seem to
 >prevent DROP EXTENSION, but dependencies on child partition indexes
 >do. ...

I agree, it looks strange ...


With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Sun Apr 19, 2026 at 9:04 PM -03, Dmitry Koval wrote:
> Hi Matheus!
>
>  >v3-0001-Preserve-extension-dependencies-on-indexes-during.patch
>
> I looked patch and did not find problems. But there is one point:
> extensions btree_gist, btree_gin, citext are not included by default.
> So command
>
>  > ./configure --enable-debug --enable-cassert --prefix `pwd`/install
>  > >/dev/null && make -s && make install -s && make check
>
> generates errors like
>
> ERROR:  extension "btree_gist" is not available
> ERROR:  extension "btree_gin" is not available
> ERROR:  extension "citext" is not available
>
> Might be it would be better to use for tests extensions from the
> catalog src/test/modules/test_extensions (see test
> src/test/modules/test_extensions/sql/test_extdepend.sql)?
>

Thanks for looking at this!

Yeah, some build farm animals will not be happy with these new tests.
Fixed on new attached v4 to use extensions from
src/test/modules/test_extensions.

>  >extension dependencies on parent partitioned indexes don't seem to
>  >prevent DROP EXTENSION, but dependencies on child partition indexes
>  >do. ...
>
> I agree, it looks strange ...
>

I'll start a new thread to discuss this.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Alexander Korotkov
Дата:
On Thu, Apr 16, 2026 at 9:03 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
>
> On Tue Apr 14, 2026 at 6:05 AM -03, Dmitry Koval wrote:
> > Hi Matheus!
> >
> > Thank you for patch.
> > I agree that dependency should be automatically added for SPLIT
> > PARTITION. But I'm not sure about MERGE PARTITION ...
> > Might be it would be more correct to automatically add a dependency only
> > if all merged partitions have it?
>
> Hi,
>
> Thank you for taking a look on this!
>
> I agree with your suggestion. The attached patch implements the
> intersection behavior for MERGE PARTITIONS: extension dependencies are
> only preserved on the merged partition's index if all source partition
> indexes have that dependency.
>
> For example:
> MERGE(idx1(ext_a, ext_b), idx2(ext_a)) -> idx3(ext_a)  -- only ext_a is common
> MERGE(idx1(ext_a), idx2())             -> idx3()       -- no common deps

This is not obvious for me.  I would rather trigger an error if there
are different dependencies on merging partitions.

> For SPLIT PARTITION, the behavior remains the same since there's only
> one source partition, all its extension dependencies are copied to the
> new partition indexes.
>
> While working on this patch, I noticed what might be a separate bug (or
> perhaps intentional behavior that I don't understand): extension
> dependencies on parent partitioned indexes don't seem to prevent DROP
> EXTENSION, but dependencies on child partition indexes do. See this
> example:
>
> CREATE EXTENSION IF NOT EXISTS btree_gist;
>
> CREATE TABLE t (i int) PARTITION BY RANGE (i);
> CREATE TABLE t_1 PARTITION OF t FOR VALUES FROM (1) TO (2);
> CREATE INDEX t_idx ON t USING gist (i);
>
> -- Add dependency on the PARENT partitioned index
> ALTER INDEX t_idx DEPENDS ON EXTENSION btree_gist;
>
> -- This succeeds (I expected it to fail):
> DROP EXTENSION btree_gist;
>
> But if I add the dependency on the child partition index instead:
>
> ALTER INDEX t_1_i_idx DEPENDS ON EXTENSION btree_gist;
>
> DROP EXTENSION btree_gist;
> ERROR:  cannot drop extension btree_gist because other objects depend on it
> DETAIL:  index t_idx depends on operator class gist_int4_ops for access method gist
>
> Is this expected behavior, or a separate bug? I would have expected the
> dependency on the parent index to also prevent the DROP.

Note that if you don't create explicit dependency then you would get
an error.  When you create an explicit dependency index => extension,
it's a different kind than the automatic one.  It causes index to be
dropped on extension drop (as documented).  Thus, if you create
dependency child_index => extension, then child_index gets dropped,
but parent_index automatic dependency still prevents deletion.  But if
you create dependency parent_index => extension then dropping of
extension trigger dropping of the parent_index, in turn that dropping
child_index.  So, everything is dropped, no errors.  I don't think
there is a bug.

------
Regards,
Alexander Korotkov
Supabase



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Mon Apr 20, 2026 at 4:08 PM -03, Alexander Korotkov wrote:
> On Thu, Apr 16, 2026 at 9:03 PM Matheus Alcantara
> <matheusssilv97@gmail.com> wrote:
>>
>> On Tue Apr 14, 2026 at 6:05 AM -03, Dmitry Koval wrote:
>> > Hi Matheus!
>> >
>> > Thank you for patch.
>> > I agree that dependency should be automatically added for SPLIT
>> > PARTITION. But I'm not sure about MERGE PARTITION ...
>> > Might be it would be more correct to automatically add a dependency only
>> > if all merged partitions have it?
>>
>> Hi,
>>
>> Thank you for taking a look on this!
>>
>> I agree with your suggestion. The attached patch implements the
>> intersection behavior for MERGE PARTITIONS: extension dependencies are
>> only preserved on the merged partition's index if all source partition
>> indexes have that dependency.
>>
>> For example:
>> MERGE(idx1(ext_a, ext_b), idx2(ext_a)) -> idx3(ext_a)  -- only ext_a is common
>> MERGE(idx1(ext_a), idx2())             -> idx3()       -- no common deps
>
> This is not obvious for me.  I would rather trigger an error if there
> are different dependencies on merging partitions.
>

Yeah, I agree that this sounds a bit confusing, although this behavior
is documented on the last patch version I think that raising an error is
more simple and maybe is more obvious. The attached patch implement
this.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Alexander Korotkov
Дата:
Hi!

On Tue, Apr 21, 2026 at 4:20 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
> On Mon Apr 20, 2026 at 4:08 PM -03, Alexander Korotkov wrote:
> > On Thu, Apr 16, 2026 at 9:03 PM Matheus Alcantara
> > <matheusssilv97@gmail.com> wrote:
> >>
> >> On Tue Apr 14, 2026 at 6:05 AM -03, Dmitry Koval wrote:
> >> > Hi Matheus!
> >> >
> >> > Thank you for patch.
> >> > I agree that dependency should be automatically added for SPLIT
> >> > PARTITION. But I'm not sure about MERGE PARTITION ...
> >> > Might be it would be more correct to automatically add a dependency only
> >> > if all merged partitions have it?
> >>
> >> Hi,
> >>
> >> Thank you for taking a look on this!
> >>
> >> I agree with your suggestion. The attached patch implements the
> >> intersection behavior for MERGE PARTITIONS: extension dependencies are
> >> only preserved on the merged partition's index if all source partition
> >> indexes have that dependency.
> >>
> >> For example:
> >> MERGE(idx1(ext_a, ext_b), idx2(ext_a)) -> idx3(ext_a)  -- only ext_a is common
> >> MERGE(idx1(ext_a), idx2())             -> idx3()       -- no common deps
> >
> > This is not obvious for me.  I would rather trigger an error if there
> > are different dependencies on merging partitions.
> >
>
> Yeah, I agree that this sounds a bit confusing, although this behavior
> is documented on the last patch version I think that raising an error is
> more simple and maybe is more obvious. The attached patch implement
> this.

I've spotted the following things in this patch.
1) The equality of dependencies is not fully checked.  We only check
that for each new dependency, we have the same for previous partition,
but not vise versa.
2) The complexity of dependency checking is O(n^2).
3) Usage of citext and other extensions in src/test/regress where they
might be not available.

I've revised the patch.
1) collectPartitionIndexExtDeps() is rewritten().  Now it works in
three phases: collect, sort, compare.  The comparison phase requires
strict equivalence of dependencies and doesn't depend on the order.
The complexity is now O(n * log(n)), which I think is acceptable.
2) PartitionIndexExtDepEntry struct now have indexOid.  So, on
conflict error contains both partition index names.
3) Tests moved to
src/test/modules/test_extensions/sql/test_extdepend.sql where
test_ext3/test_ext5 extensions are available.
4) More tests for different scenarios.

Could you, please, review this changes?

------
Regards,
Alexander Korotkov
Supabase



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Matheus Alcantara
Дата:
On 21/04/26 12:57, Alexander Korotkov wrote:
> I've spotted the following things in this patch.
> 1) The equality of dependencies is not fully checked.  We only check
> that for each new dependency, we have the same for previous partition,
> but not vise versa.
> 2) The complexity of dependency checking is O(n^2).
> 3) Usage of citext and other extensions in src/test/regress where they
> might be not available.
> 

Oops, I forgot to replace the citext extension on split partition tests.

> I've revised the patch.
> 1) collectPartitionIndexExtDeps() is rewritten().  Now it works in
> three phases: collect, sort, compare.  The comparison phase requires
> strict equivalence of dependencies and doesn't depend on the order.
> The complexity is now O(n * log(n)), which I think is acceptable.
> 2) PartitionIndexExtDepEntry struct now have indexOid.  So, on
> conflict error contains both partition index names.
> 3) Tests moved to
> src/test/modules/test_extensions/sql/test_extdepend.sql where
> test_ext3/test_ext5 extensions are available.
> 4) More tests for different scenarios.
> 
> Could you, please, review this changes?
> 

I think that you miss to include the patch?

--
Matheus Alcantara
EDB: https://www.enterprisedb.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Alexander Korotkov
Дата:
On Tue, Apr 21, 2026 at 10:23 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
> On 21/04/26 12:57, Alexander Korotkov wrote:
> > I've spotted the following things in this patch.
> > 1) The equality of dependencies is not fully checked.  We only check
> > that for each new dependency, we have the same for previous partition,
> > but not vise versa.
> > 2) The complexity of dependency checking is O(n^2).
> > 3) Usage of citext and other extensions in src/test/regress where they
> > might be not available.
> >
>
> Oops, I forgot to replace the citext extension on split partition tests.
>
> > I've revised the patch.
> > 1) collectPartitionIndexExtDeps() is rewritten().  Now it works in
> > three phases: collect, sort, compare.  The comparison phase requires
> > strict equivalence of dependencies and doesn't depend on the order.
> > The complexity is now O(n * log(n)), which I think is acceptable.
> > 2) PartitionIndexExtDepEntry struct now have indexOid.  So, on
> > conflict error contains both partition index names.
> > 3) Tests moved to
> > src/test/modules/test_extensions/sql/test_extdepend.sql where
> > test_ext3/test_ext5 extensions are available.
> > 4) More tests for different scenarios.
> >
> > Could you, please, review this changes?
> >
>
> I think that you miss to include the patch?

Yep, here it is.

------
Regards,
Alexander Korotkov
Supabase

Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Kirill Reshke
Дата:
On Wed, 22 Apr 2026 at 00:35, Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Tue, Apr 21, 2026 at 10:23 PM Matheus Alcantara
> <matheusssilv97@gmail.com> wrote:
> > On 21/04/26 12:57, Alexander Korotkov wrote:
> > > I've spotted the following things in this patch.
> > > 1) The equality of dependencies is not fully checked.  We only check
> > > that for each new dependency, we have the same for previous partition,
> > > but not vise versa.
> > > 2) The complexity of dependency checking is O(n^2).
> > > 3) Usage of citext and other extensions in src/test/regress where they
> > > might be not available.
> > >
> >
> > Oops, I forgot to replace the citext extension on split partition tests.
> >
> > > I've revised the patch.
> > > 1) collectPartitionIndexExtDeps() is rewritten().  Now it works in
> > > three phases: collect, sort, compare.  The comparison phase requires
> > > strict equivalence of dependencies and doesn't depend on the order.
> > > The complexity is now O(n * log(n)), which I think is acceptable.
> > > 2) PartitionIndexExtDepEntry struct now have indexOid.  So, on
> > > conflict error contains both partition index names.
> > > 3) Tests moved to
> > > src/test/modules/test_extensions/sql/test_extdepend.sql where
> > > test_ext3/test_ext5 extensions are available.
> > > 4) More tests for different scenarios.
> > >
> > > Could you, please, review this changes?
> > >
> >
> > I think that you miss to include the patch?
>
> Yep, here it is.
>
> ------
> Regards,
> Alexander Korotkov
> Supabase


Hi!
So, we only transfer dependencies on partitioned indexes when
SPLIT/MERGE partitions, not tables themselves (this is what this
thread started from).
I think this is correct.

Some minor comments:

> +-- Sanity check: the extension can't be dropped while dependencies exist.
>+DROP EXTENSION test_ext3;

This exercises something that already works on HEAD (note this is DROP
before first MERGE partition call ). Do we really need this?

>
> +-- An index created directly on a partition has no parent in the partitioned
> +-- index tree; merge must ignore such indexes (they disappear with the old
> +-- partition).
> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
> +    INTO part_extdep_merged2;
> +SELECT relname FROM pg_class
> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;

Looks like this test is also redundant? This does not test new DEPENDS ON logic.


Otherwise LGTM

--
Best regards,
Kirill Reshke



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Tue Apr 21, 2026 at 4:35 PM -03, Alexander Korotkov wrote:
>> > I've revised the patch.
>> > 1) collectPartitionIndexExtDeps() is rewritten().  Now it works in
>> > three phases: collect, sort, compare.  The comparison phase requires
>> > strict equivalence of dependencies and doesn't depend on the order.
>> > The complexity is now O(n * log(n)), which I think is acceptable.
>> > 2) PartitionIndexExtDepEntry struct now have indexOid.  So, on
>> > conflict error contains both partition index names.
>> > 3) Tests moved to
>> > src/test/modules/test_extensions/sql/test_extdepend.sql where
>> > test_ext3/test_ext5 extensions are available.
>> > 4) More tests for different scenarios.
>> >
>> > Could you, please, review this changes?
>> >
>>
>> I think that you miss to include the patch?
>
> Yep, here it is.
>

Thanks for the patch. It looks good to me, and I confirm that it fix the
issues that I miss on the previous version.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Wed Apr 22, 2026 at 7:48 AM -03, Kirill Reshke wrote:
>> +-- An index created directly on a partition has no parent in the partitioned
>> +-- index tree; merge must ignore such indexes (they disappear with the old
>> +-- partition).
>> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
>> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
>> +    INTO part_extdep_merged2;
>> +SELECT relname FROM pg_class
>> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
>
> Looks like this test is also redundant? This does not test new DEPENDS ON logic.
>

I think that this test is useful to ensure that we correctly skip such
indexes created directly on a specific partition. Perhaps we can include
an ALTER INDEX ... DEPENDS ON for this specific index to make it more
consistent with the other tests?

--
Matheus Alcantara
EDB: https://www.enterprisedb.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Kirill Reshke
Дата:
On Wed, 22 Apr 2026 at 15:58, Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
>
> On Wed Apr 22, 2026 at 7:48 AM -03, Kirill Reshke wrote:
> >> +-- An index created directly on a partition has no parent in the partitioned
> >> +-- index tree; merge must ignore such indexes (they disappear with the old
> >> +-- partition).
> >> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
> >> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
> >> +    INTO part_extdep_merged2;
> >> +SELECT relname FROM pg_class
> >> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
> >
> > Looks like this test is also redundant? This does not test new DEPENDS ON logic.
> >
>
> I think that this test is useful to ensure that we correctly skip such
> indexes created directly on a specific partition. Perhaps we can include
> an ALTER INDEX ... DEPENDS ON for this specific index to make it more
> consistent with the other tests?
>
> --
> Matheus Alcantara
> EDB: https://www.enterprisedb.com

This test is maybe useful, but this is unrelated to what this thread &
fix is about, for my taste.

-- 
Best regards,
Kirill Reshke



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
"Matheus Alcantara"
Дата:
On Wed Apr 22, 2026 at 7:59 AM -03, Kirill Reshke wrote:
> On Wed, 22 Apr 2026 at 15:58, Matheus Alcantara
> <matheusssilv97@gmail.com> wrote:
>>
>> On Wed Apr 22, 2026 at 7:48 AM -03, Kirill Reshke wrote:
>> >> +-- An index created directly on a partition has no parent in the partitioned
>> >> +-- index tree; merge must ignore such indexes (they disappear with the old
>> >> +-- partition).
>> >> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
>> >> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
>> >> +    INTO part_extdep_merged2;
>> >> +SELECT relname FROM pg_class
>> >> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
>> >
>> > Looks like this test is also redundant? This does not test new DEPENDS ON logic.
>> >
>>
>> I think that this test is useful to ensure that we correctly skip such
>> indexes created directly on a specific partition. Perhaps we can include
>> an ALTER INDEX ... DEPENDS ON for this specific index to make it more
>> consistent with the other tests?
>
> This test is maybe useful, but this is unrelated to what this thread &
> fix is about, for my taste.
>

On collectPartitionIndexExtDeps() we have:
    if (!get_rel_relispartition(indexOid))
        continue;

    parentIndexOid = get_partition_parent(indexOid, true);
    if (!OidIsValid(parentIndexOid))
        continue;

I think that this test ensure that get_rel_relispartition() check is
called, otherwise we will call get_partition_parent() with an index that
don't have parent which will fail with an elog(ERROR).

It can be unrelated but I think that it ensure correctness for this fix,
or I'm missing something here?

--
Matheus Alcantara
EDB: https://www.enterprisedb.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Alexander Korotkov
Дата:
On Wed, Apr 22, 2026 at 1:48 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
> Some minor comments:
>
> > +-- Sanity check: the extension can't be dropped while dependencies exist.
> >+DROP EXTENSION test_ext3;
>
> This exercises something that already works on HEAD (note this is DROP
> before first MERGE partition call ). Do we really need this?

I've removed this.

> > +-- An index created directly on a partition has no parent in the partitioned
> > +-- index tree; merge must ignore such indexes (they disappear with the old
> > +-- partition).
> > +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
> > +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
> > +    INTO part_extdep_merged2;
> > +SELECT relname FROM pg_class
> > +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
>
> Looks like this test is also redundant? This does not test new DEPENDS ON logic.

I've added the dependency on this index to check index disappears with
its dependency.  I think this would make this test more relevant.

Kirill, Matheus, are you ok with these change?

------
Regards,
Alexander Korotkov
Supabase

Вложения

Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Kirill Reshke
Дата:
On Wed, 22 Apr 2026 at 16:24, Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Wed, Apr 22, 2026 at 1:48 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
> > Some minor comments:
> >
> > > +-- Sanity check: the extension can't be dropped while dependencies exist.
> > >+DROP EXTENSION test_ext3;
> >
> > This exercises something that already works on HEAD (note this is DROP
> > before first MERGE partition call ). Do we really need this?
>
> I've removed this.
>
> > > +-- An index created directly on a partition has no parent in the partitioned
> > > +-- index tree; merge must ignore such indexes (they disappear with the old
> > > +-- partition).
> > > +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
> > > +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
> > > +    INTO part_extdep_merged2;
> > > +SELECT relname FROM pg_class
> > > +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
> >
> > Looks like this test is also redundant? This does not test new DEPENDS ON logic.
>
> I've added the dependency on this index to check index disappears with
> its dependency.  I think this would make this test more relevant.
>
> Kirill, Matheus, are you ok with these change?
>
> ------
> Regards,
> Alexander Korotkov
> Supabase

v7 WFM

--
Best regards,
Kirill Reshke



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Matheus Alcantara
Дата:
On 22/04/26 08:24, Alexander Korotkov wrote:
>>> +-- An index created directly on a partition has no parent in the partitioned
>>> +-- index tree; merge must ignore such indexes (they disappear with the old
>>> +-- partition).
>>> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
>>> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
>>> +    INTO part_extdep_merged2;
>>> +SELECT relname FROM pg_class
>>> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
>>
>> Looks like this test is also redundant? This does not test new DEPENDS ON logic.
> 
> I've added the dependency on this index to check index disappears with
> its dependency.  I think this would make this test more relevant.
> 
> Kirill, Matheus, are you ok with these change?
> 
It works for me.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Alexander Korotkov
Дата:
On Wed, Apr 22, 2026 at 2:30 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
> On 22/04/26 08:24, Alexander Korotkov wrote:
> >>> +-- An index created directly on a partition has no parent in the partitioned
> >>> +-- index tree; merge must ignore such indexes (they disappear with the old
> >>> +-- partition).
> >>> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
> >>> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
> >>> +    INTO part_extdep_merged2;
> >>> +SELECT relname FROM pg_class
> >>> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
> >>
> >> Looks like this test is also redundant? This does not test new DEPENDS ON logic.
> >
> > I've added the dependency on this index to check index disappears with
> > its dependency.  I think this would make this test more relevant.
> >
> > Kirill, Matheus, are you ok with these change?
> >
> It works for me.

Thank you, pushed!

------
Regards,
Alexander Korotkov
Supabase



Re: MERGE PARTITIONS and DEPENDS ON EXTENSION.

От
Matheus Alcantara
Дата:
On 22/04/26 08:34, Alexander Korotkov wrote:
> On Wed, Apr 22, 2026 at 2:30 PM Matheus Alcantara
> <matheusssilv97@gmail.com> wrote:
>> On 22/04/26 08:24, Alexander Korotkov wrote:
>>>>> +-- An index created directly on a partition has no parent in the partitioned
>>>>> +-- index tree; merge must ignore such indexes (they disappear with the old
>>>>> +-- partition).
>>>>> +CREATE INDEX part_extdep_3_extra_idx ON part_extdep_3(x);
>>>>> +ALTER TABLE part_extdep MERGE PARTITIONS (part_extdep_merged, part_extdep_3)
>>>>> +    INTO part_extdep_merged2;
>>>>> +SELECT relname FROM pg_class
>>>>> +WHERE relname LIKE 'part_extdep_merged2%idx' ORDER BY relname;
>>>>
>>>> Looks like this test is also redundant? This does not test new DEPENDS ON logic.
>>>
>>> I've added the dependency on this index to check index disappears with
>>> its dependency.  I think this would make this test more relevant.
>>>
>>> Kirill, Matheus, are you ok with these change?
>>>
>> It works for me.
> 
> Thank you, pushed!
> 

Thank you!

--
Matheus Alcantara
EDB: https://www.enterprisedb.com