public inbox for [email protected]  
help / color / mirror / Atom feed
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
33+ messages / 8 participants
[nested] [flat]

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-05-08 21:37 Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Justin Pryzby @ 2024-05-08 21:37 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Wed, May 08, 2024 at 09:00:10PM +0300, Alexander Korotkov wrote:
> On Fri, May 3, 2024 at 4:32 PM Alexander Korotkov <[email protected]> wrote:
> > On Fri, May 3, 2024 at 4:23 PM Justin Pryzby <[email protected]> wrote:
> > > On Wed, May 01, 2024 at 10:51:24PM +0300, Dmitry Koval wrote:
> > > > 30.04.2024 23:15, Justin Pryzby пишет:
> > > > > Is this issue already fixed ?
> > > > > I wasn't able to reproduce it.  Maybe it only happened with earlier
> > > > > patch versions applied ?
> > > >
> > > > I think this was fixed in commit [1].
> > > >
> > > > [1] https://github.com/postgres/postgres/commit/fcf80c5d5f0f3787e70fca8fd029d2e08a923f91
> > >
> > > I tried to reproduce it at fcf80c5d5f~, but couldn't.
> > > I don't see how that patch would fix it anyway.
> > > I'm hoping Alexander can confirm what happened.
> >
> > This problem is only relevant for an old version of fix [1], which
> > overrides schemas for new partitions.  That version was never
> > committed.
> 
> Here are the patches.
> 0002 Skips copying extended statistics while creating new partitions in MERGE/SPLIT
> 
> For 0002 I'd like to hear some feedback on wordings used in docs and comments.

commit message:

Currenlty => Currently
partiions => partitios
copying => by copying

> However, parent's table extended statistics already covers all its
> children.

=> That's the wrong explanation.  It's not that "stats on the parent
table cover its children".  It's that there are two types of stats:
stats for the "table hierarchy" and stats for the individual table.
That's true for single-column stats as well as for extended stats.
In both cases, that's indicated by the inh flag in the code and in the
catalog.

The right explanation is that extended stats on partitioned tables are
not similar to indexes.  Indexes on parent table are nothing other than
a mechanism to create indexes on the child tables.  That's not true for
stats.

See also my prior messages
ZiJW1g2nbQs9ekwK@pryzbyj2023
Zi5Msg74C61DjJKW@pryzbyj2023

I think EXCLUDE IDENTITY can/should now also be removed - see 509199587.
I'm not able to reproduce that problem anyway, even before that...

-- 
Justin






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
@ 2024-05-08 21:51 ` Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2024-05-08 21:51 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Thu, May 9, 2024 at 12:37 AM Justin Pryzby <[email protected]> wrote:
>
> On Wed, May 08, 2024 at 09:00:10PM +0300, Alexander Korotkov wrote:
> > On Fri, May 3, 2024 at 4:32 PM Alexander Korotkov <[email protected]> wrote:
> > > On Fri, May 3, 2024 at 4:23 PM Justin Pryzby <[email protected]> wrote:
> > > > On Wed, May 01, 2024 at 10:51:24PM +0300, Dmitry Koval wrote:
> > > > > 30.04.2024 23:15, Justin Pryzby пишет:
> > > > > > Is this issue already fixed ?
> > > > > > I wasn't able to reproduce it.  Maybe it only happened with earlier
> > > > > > patch versions applied ?
> > > > >
> > > > > I think this was fixed in commit [1].
> > > > >
> > > > > [1] https://github.com/postgres/postgres/commit/fcf80c5d5f0f3787e70fca8fd029d2e08a923f91
> > > >
> > > > I tried to reproduce it at fcf80c5d5f~, but couldn't.
> > > > I don't see how that patch would fix it anyway.
> > > > I'm hoping Alexander can confirm what happened.
> > >
> > > This problem is only relevant for an old version of fix [1], which
> > > overrides schemas for new partitions.  That version was never
> > > committed.
> >
> > Here are the patches.
> > 0002 Skips copying extended statistics while creating new partitions in MERGE/SPLIT
> >
> > For 0002 I'd like to hear some feedback on wordings used in docs and comments.
>
> commit message:
>
> Currenlty => Currently
> partiions => partitios
> copying => by copying


Thank you!

>
> > However, parent's table extended statistics already covers all its
> > children.
>
> => That's the wrong explanation.  It's not that "stats on the parent
> table cover its children".  It's that there are two types of stats:
> stats for the "table hierarchy" and stats for the individual table.
> That's true for single-column stats as well as for extended stats.
> In both cases, that's indicated by the inh flag in the code and in the
> catalog.
>
> The right explanation is that extended stats on partitioned tables are
> not similar to indexes.  Indexes on parent table are nothing other than
> a mechanism to create indexes on the child tables.  That's not true for
> stats.
>
> See also my prior messages
> ZiJW1g2nbQs9ekwK@pryzbyj2023
> Zi5Msg74C61DjJKW@pryzbyj2023

Yes, I understand that parents pg_statistic entry with stainherit ==
true includes statistics for the children.  I tried to express this by
word "covers".  But you're right, this is the wrong explanation.

Can I, please, ask you to revise the patch?

> I think EXCLUDE IDENTITY can/should now also be removed - see 509199587.
> I'm not able to reproduce that problem anyway, even before that...

I will check this.

------
Regards,
Alexander Korotkov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2024-05-14 14:49   ` Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Justin Pryzby @ 2024-05-14 14:49 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Thu, May 09, 2024 at 12:51:32AM +0300, Alexander Korotkov wrote:
> > > However, parent's table extended statistics already covers all its
> > > children.
> >
> > => That's the wrong explanation.  It's not that "stats on the parent
> > table cover its children".  It's that there are two types of stats:
> > stats for the "table hierarchy" and stats for the individual table.
> > That's true for single-column stats as well as for extended stats.
> > In both cases, that's indicated by the inh flag in the code and in the
> > catalog.
> >
> > The right explanation is that extended stats on partitioned tables are
> > not similar to indexes.  Indexes on parent table are nothing other than
> > a mechanism to create indexes on the child tables.  That's not true for
> > stats.
> >
> > See also my prior messages
> > ZiJW1g2nbQs9ekwK@pryzbyj2023
> > Zi5Msg74C61DjJKW@pryzbyj2023
> 
> Yes, I understand that parents pg_statistic entry with stainherit ==
> true includes statistics for the children.  I tried to express this by
> word "covers".  But you're right, this is the wrong explanation.
> 
> Can I, please, ask you to revise the patch?

I tried to make this clear but it'd be nice if someone (Tomas/Alvaro?)
would check that this says what's wanted.

-- 
Justin


Attachments:

  [text/x-diff] 0001-Don-t-copy-extended-statistics-during-MERGE-SPLIT-pa.patch (4.8K, ../../ZkN6EfjQGz8TOhbL@pryzbyj2023/2-0001-Don-t-copy-extended-statistics-during-MERGE-SPLIT-pa.patch)
  download | inline diff:
From 265207e5bdb215600ce5d7b45f627bc41fc2bc26 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Wed, 8 May 2024 20:32:20 +0300
Subject: [PATCH] Don't copy extended statistics during MERGE/SPLIT partition
 operations

When MERGE/SPLIT created new partitions, it was cloning the extended
statistics of the parent table.

However, extended stats on partitioned tables are not analgous to
indexes on partitioned tables (which exist only to create physical
indexes on child tables).  Rather, extended stats on a parent 1) cause
extended stats to be collected and computed across the whole partition
heirarchy, and 2) do not cause extended stats to be computed for the
individual partitions.

"CREATE TABLE ... PARTITION OF" command doesn't copy extended
statistics.  This commit makes createPartitionTable() behave
consistently.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZiJW1g2nbQs9ekwK%40pryzbyj2023
---
 doc/src/sgml/ref/alter_table.sgml | 9 +++++++--
 src/backend/commands/tablecmds.c  | 8 +++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 891fa4a7a04..313c722ee7f 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1154,9 +1154,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
      <para>
       The new partitions will be created the same as tables created with the
-      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_nameN</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)</literal>.
+      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_nameN</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)</literal>.
       The indexes and identity are created later, after moving the data
       into the new partitions.
+      Extended statistics aren't copied from the parent table, for consistency with
+      <command>CREATE TABLE PARTITION OF</command>.
+
       New partitions will have the same table access method as the parent.
       If the parent table is persistent then new partitions are created
       persistent.  If the parent table is temporary then new partitions
@@ -1224,9 +1227,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
      <para>
       The new partition will be created the same as a table created with the
-      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_name</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)</literal>.
+      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_name</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)</literal>.
       The indexes and identity are created later, after moving the data
       into the new partition.
+      Extended statistics aren't copied from the parent table, for consistency with
+      <command>CREATE TABLE PARTITION OF</command>.
       The new partition will have the same table access method as the parent.
       If the parent table is persistent then the new partition is created
       persistent.  If the parent table is temporary then the new partition
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 79c9c031833..50fc54cb309 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20419,7 +20419,7 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
  * (newPartName) like table (modelRel)
  *
  * Emulates command: CREATE [TEMP] TABLE <newPartName> (LIKE <modelRel's name>
- * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
+ * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)
  *
  * Also, this function sets the new partition access method same as parent
  * table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
@@ -20463,9 +20463,11 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 
 	/*
 	 * Indexes will be inherited on "attach new partitions" stage, after data
-	 * moving.
+	 * moving.  We also don't copy the extended statistics for consistency
+	 * with CREATE TABLE PARTITION OF.
 	 */
-	tlc->options = CREATE_TABLE_LIKE_ALL & ~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY);
+	tlc->options = CREATE_TABLE_LIKE_ALL &
+		~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
 	tlc->relationOid = InvalidOid;
 	createStmt->tableElts = lappend(createStmt->tableElts, tlc);
 
-- 
2.42.0



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
@ 2024-05-17 10:05     ` Alexander Korotkov <[email protected]>
  2024-05-17 11:02       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  0 siblings, 2 replies; 33+ messages in thread

From: Alexander Korotkov @ 2024-05-17 10:05 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Tue, May 14, 2024 at 5:49 PM Justin Pryzby <[email protected]> wrote:
> On Thu, May 09, 2024 at 12:51:32AM +0300, Alexander Korotkov wrote:
> > > > However, parent's table extended statistics already covers all its
> > > > children.
> > >
> > > => That's the wrong explanation.  It's not that "stats on the parent
> > > table cover its children".  It's that there are two types of stats:
> > > stats for the "table hierarchy" and stats for the individual table.
> > > That's true for single-column stats as well as for extended stats.
> > > In both cases, that's indicated by the inh flag in the code and in the
> > > catalog.
> > >
> > > The right explanation is that extended stats on partitioned tables are
> > > not similar to indexes.  Indexes on parent table are nothing other than
> > > a mechanism to create indexes on the child tables.  That's not true for
> > > stats.
> > >
> > > See also my prior messages
> > > ZiJW1g2nbQs9ekwK@pryzbyj2023
> > > Zi5Msg74C61DjJKW@pryzbyj2023
> >
> > Yes, I understand that parents pg_statistic entry with stainherit ==
> > true includes statistics for the children.  I tried to express this by
> > word "covers".  But you're right, this is the wrong explanation.
> >
> > Can I, please, ask you to revise the patch?
>
> I tried to make this clear but it'd be nice if someone (Tomas/Alvaro?)
> would check that this says what's wanted.

Thank you!

I've assembled the patches with the pending fixes.
0001 – The patch by Dmitry Koval for fixing detection of name
collision in SPLIT partition operation.  Also, I found that name
collision detection doesn't work well for MERGE partitions.  I've
added fix for that to this patch as well.
0002 -– Patch for skipping copy of extended statistics.  I would
appreciate more feedback about wording, but I'd like to get a correct
behavior into the source tree sooner.  If the docs and/or comments
need further improvements, we can fix that later.

I'm going to push both if no objections.

Links.
1. https://www.postgresql.org/message-id/147426d9-b793-4571-a5e5-7438affeeb5a%40postgrespro.ru

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v2-0001-Fix-the-name-collision-detection-in-MERGE-SPLIT-p.patch (8.4K, ../../CAPpHfdvz3S7SoRiaSk9C-Juy2EzHs6NC_c9uUR0i7bymKXkvuQ@mail.gmail.com/2-v2-0001-Fix-the-name-collision-detection-in-MERGE-SPLIT-p.patch)
  download | inline diff:
From eb21dcdba3263bc4702567b0af18e3ba32c4a280 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 17 May 2024 12:44:09 +0300
Subject: [PATCH v2 1/2] Fix the name collision detection in MERGE/SPLIT
 partition operations

Both MERGE and SPLIT partition operations support the case when the name of the
new partition matches the name of the existing partition to be merged/split.
But the name collision detection doesn't always work as intended.  The SPLIT
partition operation finds the namespace to search for an existing partition
without taking into account the parent's persistence.  The MERGE partition
operation checks for the name collision with simple equal() on RangeVar's
simply ignoring the search_path.

This commit fixes this behavior as follows.
 1. The SPLIT partition operation now finds the namespace to search for
    an existing partition according to the parent's persistence.
 2. The MERGE partition operation now checks for the name collision similarly
    to the SPLIT partition operation using
    RangeVarGetAndCheckCreationNamespace() and get_relname_relid().

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com
Author: Dmitry Koval, Alexander Korotkov
---
 src/backend/commands/tablecmds.c              | 62 +++++++++++++++----
 src/test/regress/expected/partition_merge.out |  3 +-
 src/test/regress/expected/partition_split.out |  8 +++
 src/test/regress/sql/partition_merge.sql      |  3 +-
 src/test/regress/sql/partition_split.sql      |  9 +++
 5 files changed, 72 insertions(+), 13 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 313c782cae2..7a063ca8ae0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20409,6 +20409,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		 * against concurrent drop, and mark stmt->relation as
 		 * RELPERSISTENCE_TEMP if a temporary namespace is selected.
 		 */
+		sps->name->relpersistence = rel->rd_rel->relpersistence;
 		namespaceId =
 			RangeVarGetAndCheckCreationNamespace(sps->name, NoLock, NULL);
 
@@ -20601,6 +20602,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	ListCell   *listptr;
 	List	   *mergingPartitionsList = NIL;
 	Oid			defaultPartOid;
+	Oid			namespaceId;
+	Oid			existingRelid;
 
 	/*
 	 * Lock all merged partitions, check them and create list with partitions
@@ -20617,13 +20620,48 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		 */
 		mergingPartition = table_openrv(name, AccessExclusiveLock);
 
-		/*
-		 * Checking that two partitions have the same name was before, in
-		 * function transformPartitionCmdForMerge().
-		 */
-		if (equal(name, cmd->name))
+		/* Store a next merging partition into the list. */
+		mergingPartitionsList = lappend(mergingPartitionsList,
+										mergingPartition);
+	}
+
+	/*
+	 * Look up the namespace in which we are supposed to create the partition,
+	 * check we have permission to create there, lock it against concurrent
+	 * drop, and mark stmt->relation as RELPERSISTENCE_TEMP if a temporary
+	 * namespace is selected.
+	 */
+	cmd->name->relpersistence = rel->rd_rel->relpersistence;
+	namespaceId =
+		RangeVarGetAndCheckCreationNamespace(cmd->name, NoLock, NULL);
+
+	/*
+	 * Check if this name is already taken.  This helps us to detect the
+	 * situation when one of the merging partitions has the same name as the
+	 * new partition.  Otherwise, this would fail later on anyway but catching
+	 * this here allows us to emit a nicer error message.
+	 */
+	existingRelid = get_relname_relid(cmd->name->relname, namespaceId);
+
+	if (OidIsValid(existingRelid))
+	{
+		Relation	sameNamePartition = NULL;
+
+		foreach_ptr(RelationData, mergingPartition, mergingPartitionsList)
 		{
-			/* One new partition can have the same name as merged partition. */
+			if (RelationGetRelid(mergingPartition) == existingRelid)
+			{
+				sameNamePartition = mergingPartition;
+				break;
+			}
+		}
+
+		if (sameNamePartition)
+		{
+			/*
+			 * The new partition has the same name as one of merging
+			 * partitions.
+			 */
 			char		tmpRelName[NAMEDATALEN];
 
 			/* Generate temporary name. */
@@ -20635,7 +20673,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			 * in the future because we're going to eventually drop the
 			 * existing partition anyway.
 			 */
-			RenameRelationInternal(RelationGetRelid(mergingPartition),
+			RenameRelationInternal(RelationGetRelid(sameNamePartition),
 								   tmpRelName, false, false);
 
 			/*
@@ -20644,10 +20682,12 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			 */
 			CommandCounterIncrement();
 		}
-
-		/* Store a next merging partition into the list. */
-		mergingPartitionsList = lappend(mergingPartitionsList,
-										mergingPartition);
+		else
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_DUPLICATE_TABLE),
+					 errmsg("relation \"%s\" already exists", cmd->name->relname)));
+		}
 	}
 
 	/* Detach all merged partitions. */
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 6361732d104..9c67a4a8b15 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -214,7 +214,8 @@ INSERT INTO sales_range VALUES (13, 'Gandi',    377,  '2022-01-09');
 INSERT INTO sales_range VALUES (14, 'Smith',    510,  '2022-05-04');
 -- Merge partitions (include DEFAULT partition) into partition with the same
 -- name
-ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022, sales_others) INTO sales_others;
+ALTER TABLE sales_range MERGE PARTITIONS
+  (sales_jan2022, sales_mar2022, partitions_merge_schema.sales_others) INTO sales_others;
 select * from sales_others;
  salesperson_id | salesperson_name | sales_amount | sales_date 
 ----------------+------------------+--------------+------------
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 14c4f97c9ff..6369f06b841 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1547,6 +1547,14 @@ REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_alice;
 REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_bob;
 DROP ROLE regress_partition_split_alice;
 DROP ROLE regress_partition_split_bob;
+-- Check that detection, that the new partition has the same name as one of
+-- the merged partitions, works correctly for temporary partitions
+CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a);
+CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2);
+ALTER TABLE t SPLIT PARTITION tp_0 INTO
+  (PARTITION tp_0 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1 FOR VALUES FROM (1) TO (2));
+DROP TABLE t;
 RESET search_path;
 --
 DROP SCHEMA partition_split_schema;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 5a741efa09b..56249732002 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -140,7 +140,8 @@ INSERT INTO sales_range VALUES (14, 'Smith',    510,  '2022-05-04');
 
 -- Merge partitions (include DEFAULT partition) into partition with the same
 -- name
-ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022, sales_others) INTO sales_others;
+ALTER TABLE sales_range MERGE PARTITIONS
+  (sales_jan2022, sales_mar2022, partitions_merge_schema.sales_others) INTO sales_others;
 
 select * from sales_others;
 
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 70d70499ec6..67ed274f529 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -931,6 +931,15 @@ REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_bob;
 DROP ROLE regress_partition_split_alice;
 DROP ROLE regress_partition_split_bob;
 
+-- Check that detection, that the new partition has the same name as one of
+-- the merged partitions, works correctly for temporary partitions
+CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a);
+CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2);
+ALTER TABLE t SPLIT PARTITION tp_0 INTO
+  (PARTITION tp_0 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1 FOR VALUES FROM (1) TO (2));
+DROP TABLE t;
+
 RESET search_path;
 
 --
-- 
2.39.3 (Apple Git-145)



  [application/octet-stream] v2-0002-Don-t-copy-extended-statistics-during-MERGE-SPLIT.patch (4.9K, ../../CAPpHfdvz3S7SoRiaSk9C-Juy2EzHs6NC_c9uUR0i7bymKXkvuQ@mail.gmail.com/3-v2-0002-Don-t-copy-extended-statistics-during-MERGE-SPLIT.patch)
  download | inline diff:
From a544336aabe5daee7e2124927de017644a01bc32 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 17 May 2024 12:56:46 +0300
Subject: [PATCH v2 2/2] Don't copy extended statistics during MERGE/SPLIT
 partition operations

When MERGE/SPLIT created new partitions, it was cloning the extended
statistics of the parent table.

However, extended stats on partitioned tables are not analgous to
indexes on partitioned tables (which exist only to create physical
indexes on child tables).  Rather, extended stats on a parent 1) cause
extended stats to be collected and computed across the whole partition
heirarchy, and 2) do not cause extended stats to be computed for the
individual partitions.

"CREATE TABLE ... PARTITION OF" command doesn't copy extended
statistics.  This commit makes createPartitionTable() behave
consistently.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZiJW1g2nbQs9ekwK%40pryzbyj2023
Author: Alexander Korotkov, Justin Pryzby
---
 doc/src/sgml/ref/alter_table.sgml | 9 +++++++--
 src/backend/commands/tablecmds.c  | 8 +++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 5d352abf991..c062a36880d 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1154,9 +1154,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
      <para>
       The new partitions will be created the same as tables created with the
-      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_nameN</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)</literal>.
+      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_nameN</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)</literal>.
       The indexes and identity are created later, after moving the data
       into the new partitions.
+      Extended statistics aren't copied from the parent table, for consistency with
+      <command>CREATE TABLE PARTITION OF</command>.
+
       New partitions will have the same table access method as the parent.
       If the parent table is persistent then new partitions are created
       persistent.  If the parent table is temporary then new partitions
@@ -1224,9 +1227,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
      <para>
       The new partition will be created the same as a table created with the
-      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_name</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)</literal>.
+      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_name</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)</literal>.
       The indexes and identity are created later, after moving the data
       into the new partition.
+      Extended statistics aren't copied from the parent table, for consistency with
+      <command>CREATE TABLE PARTITION OF</command>.
       The new partition will have the same table access method as the parent.
       If the parent table is persistent then the new partition is created
       persistent.  If the parent table is temporary then the new partition
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a063ca8ae0..7b6c69b7a52 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20269,7 +20269,7 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
  * (newPartName) like table (modelRel)
  *
  * Emulates command: CREATE [TEMP] TABLE <newPartName> (LIKE <modelRel's name>
- * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
+ * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)
  *
  * Also, this function sets the new partition access method same as parent
  * table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
@@ -20313,9 +20313,11 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 
 	/*
 	 * Indexes will be inherited on "attach new partitions" stage, after data
-	 * moving.
+	 * moving.  We also don't copy the extended statistics for consistency
+	 * with CREATE TABLE PARTITION OF.
 	 */
-	tlc->options = CREATE_TABLE_LIKE_ALL & ~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY);
+	tlc->options = CREATE_TABLE_LIKE_ALL &
+		~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
 	tlc->relationOid = InvalidOid;
 	createStmt->tableElts = lappend(createStmt->tableElts, tlc);
 
-- 
2.39.3 (Apple Git-145)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2024-05-17 11:02       ` Pavel Borisov <[email protected]>
  2024-05-17 11:33         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2024-05-17 11:02 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Alexander:

On Fri, 17 May 2024 at 14:05, Alexander Korotkov <[email protected]>
wrote:

> On Tue, May 14, 2024 at 5:49 PM Justin Pryzby <[email protected]>
> wrote:
> > On Thu, May 09, 2024 at 12:51:32AM +0300, Alexander Korotkov wrote:
> > > > > However, parent's table extended statistics already covers all its
> > > > > children.
> > > >
> > > > => That's the wrong explanation.  It's not that "stats on the parent
> > > > table cover its children".  It's that there are two types of stats:
> > > > stats for the "table hierarchy" and stats for the individual table.
> > > > That's true for single-column stats as well as for extended stats.
> > > > In both cases, that's indicated by the inh flag in the code and in
> the
> > > > catalog.
> > > >
> > > > The right explanation is that extended stats on partitioned tables
> are
> > > > not similar to indexes.  Indexes on parent table are nothing other
> than
> > > > a mechanism to create indexes on the child tables.  That's not true
> for
> > > > stats.
> > > >
> > > > See also my prior messages
> > > > ZiJW1g2nbQs9ekwK@pryzbyj2023
> > > > Zi5Msg74C61DjJKW@pryzbyj2023
> > >
> > > Yes, I understand that parents pg_statistic entry with stainherit ==
> > > true includes statistics for the children.  I tried to express this by
> > > word "covers".  But you're right, this is the wrong explanation.
> > >
> > > Can I, please, ask you to revise the patch?
> >
> > I tried to make this clear but it'd be nice if someone (Tomas/Alvaro?)
> > would check that this says what's wanted.
>
> Thank you!
>
> I've assembled the patches with the pending fixes.
> 0001 – The patch by Dmitry Koval for fixing detection of name
> collision in SPLIT partition operation.  Also, I found that name
> collision detection doesn't work well for MERGE partitions.  I've
> added fix for that to this patch as well.
> 0002 -– Patch for skipping copy of extended statistics.  I would
> appreciate more feedback about wording, but I'd like to get a correct
> behavior into the source tree sooner.  If the docs and/or comments
> need further improvements, we can fix that later.
>
> I'm going to push both if no objections.
>
Thank you for working on this patch set!

Some minor things:
0001:
partition_split.sql
157 +-- Check that detection, that the new partition has the same name as
one of
158 +-- the merged partitions, works correctly for temporary partitions
Test for split with comment for merge. Maybe better something like:
"Split partition of a temporary table when one of the partitions after
split has the same name as the partition being split"

0002:
analgous -> analogous (maybe better using "like" instead of "analogous to")
heirarchy -> hierarchy

alter_table.sgml:
Maybe in documentation it's better not to provide reasoning, just state how
it works:
for consistency with <command>CREATE TABLE PARTITION OF</command> ->
similar to <command>CREATE TABLE PARTITION OF</command>

Regards,
Pavel Borisov


^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-17 11:02       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2024-05-17 11:33         ` Alexander Korotkov <[email protected]>
  2024-05-24 19:29           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Tom Lane <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2024-05-17 11:33 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Pavel!

On Fri, May 17, 2024 at 2:02 PM Pavel Borisov <[email protected]> wrote:
> On Fri, 17 May 2024 at 14:05, Alexander Korotkov <[email protected]> wrote:
>>
>> On Tue, May 14, 2024 at 5:49 PM Justin Pryzby <[email protected]> wrote:
>> > On Thu, May 09, 2024 at 12:51:32AM +0300, Alexander Korotkov wrote:
>> > > > > However, parent's table extended statistics already covers all its
>> > > > > children.
>> > > >
>> > > > => That's the wrong explanation.  It's not that "stats on the parent
>> > > > table cover its children".  It's that there are two types of stats:
>> > > > stats for the "table hierarchy" and stats for the individual table.
>> > > > That's true for single-column stats as well as for extended stats.
>> > > > In both cases, that's indicated by the inh flag in the code and in the
>> > > > catalog.
>> > > >
>> > > > The right explanation is that extended stats on partitioned tables are
>> > > > not similar to indexes.  Indexes on parent table are nothing other than
>> > > > a mechanism to create indexes on the child tables.  That's not true for
>> > > > stats.
>> > > >
>> > > > See also my prior messages
>> > > > ZiJW1g2nbQs9ekwK@pryzbyj2023
>> > > > Zi5Msg74C61DjJKW@pryzbyj2023
>> > >
>> > > Yes, I understand that parents pg_statistic entry with stainherit ==
>> > > true includes statistics for the children.  I tried to express this by
>> > > word "covers".  But you're right, this is the wrong explanation.
>> > >
>> > > Can I, please, ask you to revise the patch?
>> >
>> > I tried to make this clear but it'd be nice if someone (Tomas/Alvaro?)
>> > would check that this says what's wanted.
>>
>> Thank you!
>>
>> I've assembled the patches with the pending fixes.
>> 0001 – The patch by Dmitry Koval for fixing detection of name
>> collision in SPLIT partition operation.  Also, I found that name
>> collision detection doesn't work well for MERGE partitions.  I've
>> added fix for that to this patch as well.
>> 0002 -– Patch for skipping copy of extended statistics.  I would
>> appreciate more feedback about wording, but I'd like to get a correct
>> behavior into the source tree sooner.  If the docs and/or comments
>> need further improvements, we can fix that later.
>>
>> I'm going to push both if no objections.
>
> Thank you for working on this patch set!
>
> Some minor things:
> 0001:
> partition_split.sql
> 157 +-- Check that detection, that the new partition has the same name as one of
> 158 +-- the merged partitions, works correctly for temporary partitions
> Test for split with comment for merge. Maybe better something like:
> "Split partition of a temporary table when one of the partitions after split has the same name as the partition being split"

Thank you, fixed as proposed.

> 0002:
> analgous -> analogous (maybe better using "like" instead of "analogous to")
> heirarchy -> hierarchy

Changed "are not analgous to" to "don't behave like".

> alter_table.sgml:
> Maybe in documentation it's better not to provide reasoning, just state how it works:
> for consistency with <command>CREATE TABLE PARTITION OF</command> -> similar to <command>CREATE TABLE PARTITION OF</command>

I'd like to keep this.  This is the question, which should naturally
arise when you read: "Why this is not just INCLUDING ALL?"

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v3-0001-Fix-the-name-collision-detection-in-MERGE-SPLIT-p.patch (8.4K, ../../CAPpHfduYTTATcdm_gTr2O9NM+CeaEPy9WhL=ycaQUxGqcgnKvA@mail.gmail.com/2-v3-0001-Fix-the-name-collision-detection-in-MERGE-SPLIT-p.patch)
  download | inline diff:
From 6a6452a1b290edfe6da91617e4bb7ec827774ca1 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 17 May 2024 12:44:09 +0300
Subject: [PATCH v3 1/2] Fix the name collision detection in MERGE/SPLIT
 partition operations

Both MERGE and SPLIT partition operations support the case when the name of the
new partition matches the name of the existing partition to be merged/split.
But the name collision detection doesn't always work as intended.  The SPLIT
partition operation finds the namespace to search for an existing partition
without taking into account the parent's persistence.  The MERGE partition
operation checks for the name collision with simple equal() on RangeVar's
simply ignoring the search_path.

This commit fixes this behavior as follows.
 1. The SPLIT partition operation now finds the namespace to search for
    an existing partition according to the parent's persistence.
 2. The MERGE partition operation now checks for the name collision similarly
    to the SPLIT partition operation using
    RangeVarGetAndCheckCreationNamespace() and get_relname_relid().

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com
Author: Dmitry Koval, Alexander Korotkov
---
 src/backend/commands/tablecmds.c              | 62 +++++++++++++++----
 src/test/regress/expected/partition_merge.out |  3 +-
 src/test/regress/expected/partition_split.out |  8 +++
 src/test/regress/sql/partition_merge.sql      |  3 +-
 src/test/regress/sql/partition_split.sql      |  9 +++
 5 files changed, 72 insertions(+), 13 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 313c782cae2..7a063ca8ae0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20409,6 +20409,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		 * against concurrent drop, and mark stmt->relation as
 		 * RELPERSISTENCE_TEMP if a temporary namespace is selected.
 		 */
+		sps->name->relpersistence = rel->rd_rel->relpersistence;
 		namespaceId =
 			RangeVarGetAndCheckCreationNamespace(sps->name, NoLock, NULL);
 
@@ -20601,6 +20602,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	ListCell   *listptr;
 	List	   *mergingPartitionsList = NIL;
 	Oid			defaultPartOid;
+	Oid			namespaceId;
+	Oid			existingRelid;
 
 	/*
 	 * Lock all merged partitions, check them and create list with partitions
@@ -20617,13 +20620,48 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		 */
 		mergingPartition = table_openrv(name, AccessExclusiveLock);
 
-		/*
-		 * Checking that two partitions have the same name was before, in
-		 * function transformPartitionCmdForMerge().
-		 */
-		if (equal(name, cmd->name))
+		/* Store a next merging partition into the list. */
+		mergingPartitionsList = lappend(mergingPartitionsList,
+										mergingPartition);
+	}
+
+	/*
+	 * Look up the namespace in which we are supposed to create the partition,
+	 * check we have permission to create there, lock it against concurrent
+	 * drop, and mark stmt->relation as RELPERSISTENCE_TEMP if a temporary
+	 * namespace is selected.
+	 */
+	cmd->name->relpersistence = rel->rd_rel->relpersistence;
+	namespaceId =
+		RangeVarGetAndCheckCreationNamespace(cmd->name, NoLock, NULL);
+
+	/*
+	 * Check if this name is already taken.  This helps us to detect the
+	 * situation when one of the merging partitions has the same name as the
+	 * new partition.  Otherwise, this would fail later on anyway but catching
+	 * this here allows us to emit a nicer error message.
+	 */
+	existingRelid = get_relname_relid(cmd->name->relname, namespaceId);
+
+	if (OidIsValid(existingRelid))
+	{
+		Relation	sameNamePartition = NULL;
+
+		foreach_ptr(RelationData, mergingPartition, mergingPartitionsList)
 		{
-			/* One new partition can have the same name as merged partition. */
+			if (RelationGetRelid(mergingPartition) == existingRelid)
+			{
+				sameNamePartition = mergingPartition;
+				break;
+			}
+		}
+
+		if (sameNamePartition)
+		{
+			/*
+			 * The new partition has the same name as one of merging
+			 * partitions.
+			 */
 			char		tmpRelName[NAMEDATALEN];
 
 			/* Generate temporary name. */
@@ -20635,7 +20673,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			 * in the future because we're going to eventually drop the
 			 * existing partition anyway.
 			 */
-			RenameRelationInternal(RelationGetRelid(mergingPartition),
+			RenameRelationInternal(RelationGetRelid(sameNamePartition),
 								   tmpRelName, false, false);
 
 			/*
@@ -20644,10 +20682,12 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			 */
 			CommandCounterIncrement();
 		}
-
-		/* Store a next merging partition into the list. */
-		mergingPartitionsList = lappend(mergingPartitionsList,
-										mergingPartition);
+		else
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_DUPLICATE_TABLE),
+					 errmsg("relation \"%s\" already exists", cmd->name->relname)));
+		}
 	}
 
 	/* Detach all merged partitions. */
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 6361732d104..9c67a4a8b15 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -214,7 +214,8 @@ INSERT INTO sales_range VALUES (13, 'Gandi',    377,  '2022-01-09');
 INSERT INTO sales_range VALUES (14, 'Smith',    510,  '2022-05-04');
 -- Merge partitions (include DEFAULT partition) into partition with the same
 -- name
-ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022, sales_others) INTO sales_others;
+ALTER TABLE sales_range MERGE PARTITIONS
+  (sales_jan2022, sales_mar2022, partitions_merge_schema.sales_others) INTO sales_others;
 select * from sales_others;
  salesperson_id | salesperson_name | sales_amount | sales_date 
 ----------------+------------------+--------------+------------
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 14c4f97c9ff..d08eb4770ba 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1547,6 +1547,14 @@ REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_alice;
 REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_bob;
 DROP ROLE regress_partition_split_alice;
 DROP ROLE regress_partition_split_bob;
+-- Split partition of a temporary table when one of the partitions after
+-- split has the same name as the partition being split
+CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a);
+CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2);
+ALTER TABLE t SPLIT PARTITION tp_0 INTO
+  (PARTITION tp_0 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1 FOR VALUES FROM (1) TO (2));
+DROP TABLE t;
 RESET search_path;
 --
 DROP SCHEMA partition_split_schema;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 5a741efa09b..56249732002 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -140,7 +140,8 @@ INSERT INTO sales_range VALUES (14, 'Smith',    510,  '2022-05-04');
 
 -- Merge partitions (include DEFAULT partition) into partition with the same
 -- name
-ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022, sales_others) INTO sales_others;
+ALTER TABLE sales_range MERGE PARTITIONS
+  (sales_jan2022, sales_mar2022, partitions_merge_schema.sales_others) INTO sales_others;
 
 select * from sales_others;
 
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 70d70499ec6..d9e2359cb76 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -931,6 +931,15 @@ REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_bob;
 DROP ROLE regress_partition_split_alice;
 DROP ROLE regress_partition_split_bob;
 
+-- Split partition of a temporary table when one of the partitions after
+-- split has the same name as the partition being split
+CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a);
+CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2);
+ALTER TABLE t SPLIT PARTITION tp_0 INTO
+  (PARTITION tp_0 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1 FOR VALUES FROM (1) TO (2));
+DROP TABLE t;
+
 RESET search_path;
 
 --
-- 
2.39.3 (Apple Git-145)



  [application/octet-stream] v3-0002-Don-t-copy-extended-statistics-during-MERGE-SPLIT.patch (4.9K, ../../CAPpHfduYTTATcdm_gTr2O9NM+CeaEPy9WhL=ycaQUxGqcgnKvA@mail.gmail.com/3-v3-0002-Don-t-copy-extended-statistics-during-MERGE-SPLIT.patch)
  download | inline diff:
From dc8c132f1a0d38e8535f2d0d05dcc5b33527f5af Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 17 May 2024 12:56:46 +0300
Subject: [PATCH v3 2/2] Don't copy extended statistics during MERGE/SPLIT
 partition operations

When MERGE/SPLIT created new partitions, it was cloning the extended
statistics of the parent table.

However, extended stats on partitioned tables don't behave like
indexes on partitioned tables (which exist only to create physical
indexes on child tables).  Rather, extended stats on a parent 1) cause
extended stats to be collected and computed across the whole partition
hierarchy, and 2) do not cause extended stats to be computed for the
individual partitions.

"CREATE TABLE ... PARTITION OF" command doesn't copy extended
statistics.  This commit makes createPartitionTable() behave
consistently.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZiJW1g2nbQs9ekwK%40pryzbyj2023
Author: Alexander Korotkov, Justin Pryzby
---
 doc/src/sgml/ref/alter_table.sgml | 9 +++++++--
 src/backend/commands/tablecmds.c  | 8 +++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 5d352abf991..c062a36880d 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1154,9 +1154,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
      <para>
       The new partitions will be created the same as tables created with the
-      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_nameN</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)</literal>.
+      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_nameN</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)</literal>.
       The indexes and identity are created later, after moving the data
       into the new partitions.
+      Extended statistics aren't copied from the parent table, for consistency with
+      <command>CREATE TABLE PARTITION OF</command>.
+
       New partitions will have the same table access method as the parent.
       If the parent table is persistent then new partitions are created
       persistent.  If the parent table is temporary then new partitions
@@ -1224,9 +1227,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      </para>
      <para>
       The new partition will be created the same as a table created with the
-      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_name</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)</literal>.
+      SQL command <literal>CREATE TABLE <replaceable class="parameter">partition_name</replaceable> (LIKE <replaceable class="parameter">name</replaceable> INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)</literal>.
       The indexes and identity are created later, after moving the data
       into the new partition.
+      Extended statistics aren't copied from the parent table, for consistency with
+      <command>CREATE TABLE PARTITION OF</command>.
       The new partition will have the same table access method as the parent.
       If the parent table is persistent then the new partition is created
       persistent.  If the parent table is temporary then the new partition
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a063ca8ae0..7b6c69b7a52 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20269,7 +20269,7 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
  * (newPartName) like table (modelRel)
  *
  * Emulates command: CREATE [TEMP] TABLE <newPartName> (LIKE <modelRel's name>
- * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
+ * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)
  *
  * Also, this function sets the new partition access method same as parent
  * table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
@@ -20313,9 +20313,11 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 
 	/*
 	 * Indexes will be inherited on "attach new partitions" stage, after data
-	 * moving.
+	 * moving.  We also don't copy the extended statistics for consistency
+	 * with CREATE TABLE PARTITION OF.
 	 */
-	tlc->options = CREATE_TABLE_LIKE_ALL & ~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY);
+	tlc->options = CREATE_TABLE_LIKE_ALL &
+		~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
 	tlc->relationOid = InvalidOid;
 	createStmt->tableElts = lappend(createStmt->tableElts, tlc);
 
-- 
2.39.3 (Apple Git-145)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-17 11:02       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2024-05-17 11:33         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2024-05-24 19:29           ` Tom Lane <[email protected]>
  2024-05-24 20:00             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Lakhin <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Tom Lane @ 2024-05-24 19:29 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

The partition_split test has unstable results, as shown at [1].
I suggest adding "ORDER BY conname" to the two queries shown
to fail there.  Better look at other queries in the test for
possible similar problems, too.

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jackdaw&dt=2024-05-24%2015%3A58%3A17






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-17 11:02       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2024-05-17 11:33         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-24 19:29           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Tom Lane <[email protected]>
@ 2024-05-24 20:00             ` Alexander Lakhin <[email protected]>
  2024-05-25 12:53               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Lakhin @ 2024-05-24 20:00 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hello,

24.05.2024 22:29, Tom Lane wrote:
> The partition_split test has unstable results, as shown at [1].
> I suggest adding "ORDER BY conname" to the two queries shown
> to fail there.  Better look at other queries in the test for
> possible similar problems, too.

Yes, I've just reproduced it on an aarch64 device as follows:
echo "autovacuum_naptime = 1
autovacuum_vacuum_threshold = 1
autovacuum_analyze_threshold = 1
" > ~/temp.config
TEMP_CONFIG=~/temp.config TESTS="$(printf 'partition_split %.0s' `seq 100`)" make -s check-tests
...
ok 80        - partition_split                           749 ms
not ok 81    - partition_split                           728 ms
ok 82        - partition_split                           732 ms

$ cat src/test/regress/regression.diffs
diff -U3 .../src/test/regress/expected/partition_split.out .../src/test/regress/results/partition_split.out
--- .../src/test/regress/expected/partition_split.out   2024-05-15 17:15:57.171999830 +0000
+++ .../src/test/regress/results/partition_split.out    2024-05-24 19:28:37.329999749 +0000
@@ -625,8 +625,8 @@
  SELECT pg_get_constraintdef(oid), conname, conkey FROM pg_constraint WHERE conrelid = 
'sales_feb_mar_apr2022'::regclass::oid;
pg_get_constraintdef                         | conname             | conkey
  ---------------------------------------------------------------------+---------------------------------+--------
- CHECK ((sales_amount > 1))                                          | sales_range_sales_amount_check  | {2}
   FOREIGN KEY (salesperson_id) REFERENCES salespeople(salesperson_id) | sales_range_salesperson_id_fkey | {1}
+ CHECK ((sales_amount > 1))                                          | sales_range_sales_amount_check  | {2}
  (2 rows)

  ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO

Best regards,
Alexander






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-17 11:02       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2024-05-17 11:33         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-24 19:29           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Tom Lane <[email protected]>
  2024-05-24 20:00             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Lakhin <[email protected]>
@ 2024-05-25 12:53               ` Alexander Korotkov <[email protected]>
  2024-05-26 03:58                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2024-05-25 12:53 UTC (permalink / raw)
  To: Alexander Lakhin <[email protected]>; +Cc: Tom Lane <[email protected]>; Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Fri, May 24, 2024 at 11:00 PM Alexander Lakhin <[email protected]> wrote:
>
> 24.05.2024 22:29, Tom Lane wrote:
> > The partition_split test has unstable results, as shown at [1].
> > I suggest adding "ORDER BY conname" to the two queries shown
> > to fail there.  Better look at other queries in the test for
> > possible similar problems, too.
>
> Yes, I've just reproduced it on an aarch64 device as follows:
> echo "autovacuum_naptime = 1
> autovacuum_vacuum_threshold = 1
> autovacuum_analyze_threshold = 1
> " > ~/temp.config
> TEMP_CONFIG=~/temp.config TESTS="$(printf 'partition_split %.0s' `seq 100`)" make -s check-tests
> ...
> ok 80        - partition_split                           749 ms
> not ok 81    - partition_split                           728 ms
> ok 82        - partition_split                           732 ms
>
> $ cat src/test/regress/regression.diffs
> diff -U3 .../src/test/regress/expected/partition_split.out .../src/test/regress/results/partition_split.out
> --- .../src/test/regress/expected/partition_split.out   2024-05-15 17:15:57.171999830 +0000
> +++ .../src/test/regress/results/partition_split.out    2024-05-24 19:28:37.329999749 +0000
> @@ -625,8 +625,8 @@
>   SELECT pg_get_constraintdef(oid), conname, conkey FROM pg_constraint WHERE conrelid =
> 'sales_feb_mar_apr2022'::regclass::oid;
> pg_get_constraintdef                         | conname             | conkey
>   ---------------------------------------------------------------------+---------------------------------+--------
> - CHECK ((sales_amount > 1))                                          | sales_range_sales_amount_check  | {2}
>    FOREIGN KEY (salesperson_id) REFERENCES salespeople(salesperson_id) | sales_range_salesperson_id_fkey | {1}
> + CHECK ((sales_amount > 1))                                          | sales_range_sales_amount_check  | {2}
>   (2 rows)
>
>   ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO

Tom, Alexander, thank you for spotting this.
I'm going to care about it later today.

------
Regards,
Alexander Korotkov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-17 11:02       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2024-05-17 11:33         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-24 19:29           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Tom Lane <[email protected]>
  2024-05-24 20:00             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Lakhin <[email protected]>
  2024-05-25 12:53               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2024-05-26 03:58                 ` Alexander Korotkov <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Alexander Korotkov @ 2024-05-26 03:58 UTC (permalink / raw)
  To: Alexander Lakhin <[email protected]>; +Cc: Tom Lane <[email protected]>; Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Sat, May 25, 2024 at 3:53 PM Alexander Korotkov <[email protected]> wrote:
> On Fri, May 24, 2024 at 11:00 PM Alexander Lakhin <[email protected]> wrote:
> >
> > 24.05.2024 22:29, Tom Lane wrote:
> > > The partition_split test has unstable results, as shown at [1].
> > > I suggest adding "ORDER BY conname" to the two queries shown
> > > to fail there.  Better look at other queries in the test for
> > > possible similar problems, too.
> >
> > Yes, I've just reproduced it on an aarch64 device as follows:
> > echo "autovacuum_naptime = 1
> > autovacuum_vacuum_threshold = 1
> > autovacuum_analyze_threshold = 1
> > " > ~/temp.config
> > TEMP_CONFIG=~/temp.config TESTS="$(printf 'partition_split %.0s' `seq 100`)" make -s check-tests
> > ...
> > ok 80        - partition_split                           749 ms
> > not ok 81    - partition_split                           728 ms
> > ok 82        - partition_split                           732 ms
> >
> > $ cat src/test/regress/regression.diffs
> > diff -U3 .../src/test/regress/expected/partition_split.out .../src/test/regress/results/partition_split.out
> > --- .../src/test/regress/expected/partition_split.out   2024-05-15 17:15:57.171999830 +0000
> > +++ .../src/test/regress/results/partition_split.out    2024-05-24 19:28:37.329999749 +0000
> > @@ -625,8 +625,8 @@
> >   SELECT pg_get_constraintdef(oid), conname, conkey FROM pg_constraint WHERE conrelid =
> > 'sales_feb_mar_apr2022'::regclass::oid;
> > pg_get_constraintdef                         | conname             | conkey
> >   ---------------------------------------------------------------------+---------------------------------+--------
> > - CHECK ((sales_amount > 1))                                          | sales_range_sales_amount_check  | {2}
> >    FOREIGN KEY (salesperson_id) REFERENCES salespeople(salesperson_id) | sales_range_salesperson_id_fkey | {1}
> > + CHECK ((sales_amount > 1))                                          | sales_range_sales_amount_check  | {2}
> >   (2 rows)
> >
> >   ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
>
> Tom, Alexander, thank you for spotting this.
> I'm going to care about it later today.

ORDER BY is added in d53a4286d7 in these queries altogether with other
catalog queries with potentially unstable result.

------
Regards,
Alexander Korotkov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-15 11:07       ` Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Justin Pryzby @ 2026-06-15 11:07 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

I hit an error when I tried this patch.

CREATE TABLE a (a text) PARTITION BY RANGE(a);
CREATE TABLE a1 PARTITION OF a DEFAULT;
INSERT INTO a SELECT repeat('1', 9999999);
CREATE TABLE a2 PARTITION OF a FOR VALUES FROM (2)TO(3);
ALTER TABLE a MERGE PARTITIONS (a1, a2) INTO a1;
ERROR:  row is too big: size 39264, maximum size 8160

-- 
Justin






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
@ 2026-06-15 20:50         ` Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-15 20:50 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Mon, Jun 15, 2026 at 2:07 PM Justin Pryzby <[email protected]> wrote:
>
> I hit an error when I tried this patch.
>
> CREATE TABLE a (a text) PARTITION BY RANGE(a);
> CREATE TABLE a1 PARTITION OF a DEFAULT;
> INSERT INTO a SELECT repeat('1', 9999999);
> CREATE TABLE a2 PARTITION OF a FOR VALUES FROM (2)TO(3);
> ALTER TABLE a MERGE PARTITIONS (a1, a2) INTO a1;
> ERROR:  row is too big: size 39264, maximum size 8160

Thank you for your report.  It appears that createPartitionTable()
misses NewRelationCreateToastTable() call (for instance, DefineTable()
has this call).  The attached patch implements fix and has tests.  I'm
going to push it if no objections.

------
Regards,
Alexander Korotkov
Supabase





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-15 21:36           ` Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2026-06-15 21:36 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Alexander!

On Tue, 16 Jun 2026 at 00:50, Alexander Korotkov <[email protected]> wrote:
>
> On Mon, Jun 15, 2026 at 2:07 PM Justin Pryzby <[email protected]> wrote:
> >
> > I hit an error when I tried this patch.
> >
> > CREATE TABLE a (a text) PARTITION BY RANGE(a);
> > CREATE TABLE a1 PARTITION OF a DEFAULT;
> > INSERT INTO a SELECT repeat('1', 9999999);
> > CREATE TABLE a2 PARTITION OF a FOR VALUES FROM (2)TO(3);
> > ALTER TABLE a MERGE PARTITIONS (a1, a2) INTO a1;
> > ERROR:  row is too big: size 39264, maximum size 8160
>
> Thank you for your report.  It appears that createPartitionTable()
> misses NewRelationCreateToastTable() call (for instance, DefineTable()
> has this call).  The attached patch implements fix and has tests.  I'm
> going to push it if no objections.

It looks like the patch is missing in the last message.

Regards,
Pavel Borisov,
Supabase





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2026-06-15 21:36             ` Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 08:08               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands jian he <[email protected]>
  0 siblings, 2 replies; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-15 21:36 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Tue, Jun 16, 2026 at 12:36 AM Pavel Borisov <[email protected]> wrote:
> On Tue, 16 Jun 2026 at 00:50, Alexander Korotkov <[email protected]> wrote:
> >
> > On Mon, Jun 15, 2026 at 2:07 PM Justin Pryzby <[email protected]> wrote:
> > >
> > > I hit an error when I tried this patch.
> > >
> > > CREATE TABLE a (a text) PARTITION BY RANGE(a);
> > > CREATE TABLE a1 PARTITION OF a DEFAULT;
> > > INSERT INTO a SELECT repeat('1', 9999999);
> > > CREATE TABLE a2 PARTITION OF a FOR VALUES FROM (2)TO(3);
> > > ALTER TABLE a MERGE PARTITIONS (a1, a2) INTO a1;
> > > ERROR:  row is too big: size 39264, maximum size 8160
> >
> > Thank you for your report.  It appears that createPartitionTable()
> > misses NewRelationCreateToastTable() call (for instance, DefineTable()
> > has this call).  The attached patch implements fix and has tests.  I'm
> > going to push it if no objections.
>
> It looks like the patch is missing in the last message.

Sorry, here it is.

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v1-0001-Create-TOAST-table-for-partitions-made-by-MERGE-S.patch (7.0K, ../../CAPpHfdvpzvNt=d_Csxe-TCtvF9MRNdeMtfJu3RkmTgBkkQbGUw@mail.gmail.com/2-v1-0001-Create-TOAST-table-for-partitions-made-by-MERGE-S.patch)
  download | inline diff:
From 9ee64c74480a5bb61062aa3f173f7ec98b2ce8ad Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 15 Jun 2026 15:05:23 +0300
Subject: [PATCH v1] Create TOAST table for partitions made by MERGE/SPLIT
 PARTITION

ALTER TABLE ... MERGE PARTITIONS / SPLIT PARTITION builds a new
partition via createPartitionTable(), but never gave it a TOAST table.
When the source rows carried out-of-line varlena values, the move
into the new partition entered heap_toast_insert_or_update() with
reltoastrelid = InvalidOid: the externalization step is skipped, the
value falls back to inline storage and heap_insert() fails with
"row is too big" error.  Also, TOAST table is needed if the new partition
receives out-of-line varlena values after the DDL operation is complete.

Call NewRelationCreateToastTable() right after the new partition is
created in createPartitionTable(), mirroring what DefineRelation()
does for regular CREATE TABLE.  NewRelationCreateToastTable() decides
on its own whether a TOAST table is actually required, so partitions
with no toast-eligible columns are unaffected.

Reported-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/ai_c4-v8iLA2kXFV%40pryzbyj2023
---
 src/backend/commands/tablecmds.c              |  9 ++++++
 src/test/regress/expected/partition_merge.out | 23 +++++++++++++++
 src/test/regress/expected/partition_split.out | 28 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 14 ++++++++++
 src/test/regress/sql/partition_split.sql      | 17 +++++++++++
 5 files changed, 91 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 38f9ffcd04f..265dcfe7fda 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -22815,6 +22815,15 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 	 */
 	CommandCounterIncrement();
 
+	/*
+	 * Create a TOAST table if the table needs one.  MERGE/SPLIT PARTITION
+	 * moves rows from existing partition(s) into new partition(s), which may
+	 * carry out-of-line varlena values that the new relation must be able to
+	 * store.  Also, the new partition must be able to receive out-of-line
+	 * varlena values after the DDL operation is complete.
+	 */
+	NewRelationCreateToastTable(newRelId, (Datum) 0);
+
 	/*
 	 * Open the new partition with no lock, because we already have an
 	 * AccessExclusiveLock placed there after creation.
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index d3818f1bf9b..49ffa9a0631 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -1088,6 +1088,29 @@ SELECT count(*) FROM t WHERE i = 15 AND g IN (SELECT g + 10 FROM t WHERE i = 5);
      1
 (1 row)
 
+DROP TABLE t;
+-- Merged partitions need their own TOAST table; otherwise an out-of-line
+-- varlena values can't be stored.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+CREATE TABLE tp_def PARTITION OF t DEFAULT;
+CREATE TABLE tp_2_3 PARTITION OF t FOR VALUES FROM ('2') TO ('3');
+-- Long value: out-of-line storage even after pglz compression
+INSERT INTO t SELECT repeat('1', 9999999);
+ALTER TABLE t MERGE PARTITIONS (tp_def, tp_2_3) INTO tp_merged;
+SELECT reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname = 'tp_merged';
+ has_toast | toast_used 
+-----------+------------
+ t         | t
+(1 row)
+
+SELECT length(a) FROM t;
+ length  
+---------
+ 9999999
+(1 row)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index ff6027af658..d651767c8b8 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1653,6 +1653,34 @@ SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i =
      0
 (1 row)
 
+DROP TABLE t;
+-- Each new partition produced by SPLIT must get its own TOAST table so
+-- that out-of-line varlena attributes coming from the source partition
+-- can be stored.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
+-- Long value: out-of-line storage even after pglz compression
+INSERT INTO t SELECT repeat('1', 9999999);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (MINVALUE) TO ('2'),
+    PARTITION tp_hi FOR VALUES FROM ('2') TO (MAXVALUE)
+);
+SELECT relname,
+       reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+ relname | has_toast | toast_used 
+---------+-----------+------------
+ tp_hi   | t         | f
+ tp_lo   | t         | t
+(2 rows)
+
+SELECT length(a) FROM t;
+ length  
+---------
+ 9999999
+(1 row)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 1e14ed40f5c..7a9d43552a7 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -781,6 +781,20 @@ SELECT count(*) FROM t WHERE i = 15 AND g IN (SELECT g + 10 FROM t WHERE i = 5);
 
 DROP TABLE t;
 
+-- Merged partitions need their own TOAST table; otherwise an out-of-line
+-- varlena values can't be stored.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+CREATE TABLE tp_def PARTITION OF t DEFAULT;
+CREATE TABLE tp_2_3 PARTITION OF t FOR VALUES FROM ('2') TO ('3');
+-- Long value: out-of-line storage even after pglz compression
+INSERT INTO t SELECT repeat('1', 9999999);
+ALTER TABLE t MERGE PARTITIONS (tp_def, tp_2_3) INTO tp_merged;
+SELECT reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname = 'tp_merged';
+SELECT length(a) FROM t;
+DROP TABLE t;
+
 
 RESET search_path;
 
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 05de24152d1..06636a366a5 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -1184,6 +1184,23 @@ SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i =
 
 DROP TABLE t;
 
+-- Each new partition produced by SPLIT must get its own TOAST table so
+-- that out-of-line varlena attributes coming from the source partition
+-- can be stored.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
+-- Long value: out-of-line storage even after pglz compression
+INSERT INTO t SELECT repeat('1', 9999999);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (MINVALUE) TO ('2'),
+    PARTITION tp_hi FOR VALUES FROM ('2') TO (MAXVALUE)
+);
+SELECT relname,
+       reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+SELECT length(a) FROM t;
+DROP TABLE t;
 
 RESET search_path;
 
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-15 21:56               ` Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2026-06-15 21:56 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Alexander!

I looked into the patch. It's simple enough and looks good to me.
I have one question: is it correct to pass 0 reloptions to
NewRelationCreateToastTable or we need to use transformRelOptions +
heap_reloptions pair to derive toast_reloptions as it is done in
ProcessUtilitySlow() and create_ctas_internal()?

Regards,
Pavel Borisov






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2026-06-16 12:22                 ` Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-16 12:22 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Pavel!

On Tue, Jun 16, 2026 at 12:57 AM Pavel Borisov <[email protected]> wrote:
> I looked into the patch. It's simple enough and looks good to me.
> I have one question: is it correct to pass 0 reloptions to
> NewRelationCreateToastTable or we need to use transformRelOptions +
> heap_reloptions pair to derive toast_reloptions as it is done in
> ProcessUtilitySlow() and create_ctas_internal()?

Yes, (Datum) 0 is correct here.  MERGE/SPLIT PARTITION grammar
provides no syntactic way for the user to specify reloptions for the
new partitions, so there is nothing to extract via
transformRelOptions.  This matches createPartitionTable() itself,
which already calls heap_create_with_catalog(... reloptions=(Datum) 0
...) for the main relation a few lines above; the TOAST table just
inherits that choice.

Extending the syntax to accept WITH (...) on new partitions would be a
separate, larger feature.  Out of scope for this bug fix.

------
Regards,
Alexander Korotkov
Supabase





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-16 13:01                   ` Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2026-06-16 13:01 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Alexander!

> Extending the syntax to accept WITH (...) on new partitions would be a
> separate, larger feature.  Out of scope for this bug fix.

I agree, this is out of scope of a fix.
Updated test in v2 and overall patch looks good to me.

A couple of wording proposals:
never gave it -> never gives it
Merged partitions need their own TOAST table -> A merged partition
needs its own TOAST table.
re-stored -> stored
a short string suffices -> a string over that threshold suffices

Regards,
Pavel Borisov,
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2026-06-18 07:31                     ` Alexander Korotkov <[email protected]>
  2026-06-18 15:49                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  0 siblings, 2 replies; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-18 07:31 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Pavel!

On Tue, Jun 16, 2026 at 4:01 PM Pavel Borisov <[email protected]> wrote:
> > Extending the syntax to accept WITH (...) on new partitions would be a
> > separate, larger feature.  Out of scope for this bug fix.
>
> I agree, this is out of scope of a fix.
> Updated test in v2 and overall patch looks good to me.
>
> A couple of wording proposals:
> never gave it -> never gives it
> Merged partitions need their own TOAST table -> A merged partition
> needs its own TOAST table.
> re-stored -> stored
> a short string suffices -> a string over that threshold suffices

Pushed with your suggestions accepted.

------
Regards,
Alexander Korotkov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-18 15:49                       ` Justin Pryzby <[email protected]>
  2026-06-21 12:11                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Justin Pryzby @ 2026-06-18 15:49 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Thu, Jun 18, 2026 at 10:31:11AM +0300, Alexander Korotkov wrote:
> Pushed with your suggestions accepted.

Thanks.  When I went back to test this, I merged ~25 partitions that
were all on the same tablespace, but the merged table was created on the
default tablespace.

I tried again with default_tablespace set, but it was ignored.  I think
that's wrong.  It's good to follow the tablespace of the parent table,
but if it has no tablespace set, default_tablespace should be obeyed.
See surrounding logic in DefineRelation.

I see the docs say this:
+       <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
+       table itself as the template to construct the new partition.
+       The new partition will inherit the same table access method, persistence
+       type, and tablespace as the partitioned table.

-- 
Justin






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-18 15:49                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
@ 2026-06-21 12:11                         ` Alexander Korotkov <[email protected]>
  2026-06-22 10:44                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-21 12:11 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Thu, Jun 18, 2026 at 6:49 PM Justin Pryzby <[email protected]> wrote:
> On Thu, Jun 18, 2026 at 10:31:11AM +0300, Alexander Korotkov wrote:
> > Pushed with your suggestions accepted.
>
> Thanks.  When I went back to test this, I merged ~25 partitions that
> were all on the same tablespace, but the merged table was created on the
> default tablespace.
>
> I tried again with default_tablespace set, but it was ignored.  I think
> that's wrong.  It's good to follow the tablespace of the parent table,
> but if it has no tablespace set, default_tablespace should be obeyed.
> See surrounding logic in DefineRelation.
>
> I see the docs say this:
> +       <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
> +       table itself as the template to construct the new partition.
> +       The new partition will inherit the same table access method, persistence
> +       type, and tablespace as the partitioned table.

Correct, please see the attached patch, it makes
createPartitionTable() deal with tablespaces the same was as
DefineRelation() does.

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v1-0001-Take-into-account-default_tablespace-during-MERGE.patch (15.0K, ../../CAPpHfduZ9Yd6vG0EOZUa_MOVxpEDMsZB-89pgNypwFzp2ax+NQ@mail.gmail.com/2-v1-0001-Take-into-account-default_tablespace-during-MERGE.patch)
  download | inline diff:
From e2e9bad654a2083dedae1b7420bde1036fb3eaac Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Thu, 18 Jun 2026 21:53:32 +0300
Subject: [PATCH v1] Take into account default_tablespace during MERGE/SPLIT
 PARTITION(S)

createPartitionTable() passed the partitioned parent's reltablespace straight
to heap_create_with_catalog(), bypassing the default_tablespace GUC fallback
that DefineRelation() applies for CREATE TABLE ... PARTITION OF.  When the
parent had no explicit tablespace (reltablespace = 0), the new partition
unconditionally landed in the database default, even if default_tablespace
was set to something else; merging or splitting a set of partitions that all
lived in a non-default tablespace produced a new partition in the database
default.

Mirror DefineRelation()'s logic: take parent's reltablespace if set,
otherwise check GetDefaultTablespace() (which reads default_tablespace
and normalises pg_default / MyDatabaseTableSpace to InvalidOid).  Also
add the CREATE ACL check on the resolved tablespace, matching
DefineRelation()'s behavior.

Update the documentation for MERGE/SPLIT PARTITION to spell out the
tablespace-selection rule explicitly.

Reported-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/ajQTklv8QArzTp3h%40pryzbyj2023
---
 doc/src/sgml/ref/alter_table.sgml             | 18 ++++--
 src/backend/commands/tablecmds.c              | 25 +++++++-
 src/test/regress/expected/partition_merge.out | 48 +++++++++++++++
 src/test/regress/expected/partition_split.out | 60 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 36 +++++++++++
 src/test/regress/sql/partition_split.sql      | 45 ++++++++++++++
 6 files changed, 227 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index dec34337d1a..36c840f68d8 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1233,8 +1233,13 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      <para>
        <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
        table itself as the template to construct the new partition.
-       The new partition will inherit the same table access method, persistence
-       type, and tablespace as the partitioned table.
+       The new partition inherits the table access method and persistence type
+       of the partitioned table.  Its tablespace is selected as for a
+       <command>CREATE TABLE ... PARTITION OF</command> command issued
+       without a <literal>TABLESPACE</literal> clause: if the partitioned
+       table has an explicit tablespace, the new partition uses it;
+       otherwise the value of <xref linkend="guc-default-tablespace"/> is
+       taken into account, falling back to the database's default tablespace.
 
        Constraints, column defaults, column generation expressions, identity
        columns, indexes, and triggers are copied from the partitioned table to
@@ -1331,8 +1336,13 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      <para>
        <command>ALTER TABLE SPLIT PARTITION</command> uses the partitioned
        table itself as the template to construct new partitions.
-       New partitions will inherit the same table access method, persistence
-       type, and tablespace as the partitioned table.
+       New partitions inherit the table access method and persistence type of
+       the partitioned table.  Their tablespace is selected as for a
+       <command>CREATE TABLE ... PARTITION OF</command> command issued
+       without a <literal>TABLESPACE</literal> clause: if the partitioned
+       table has an explicit tablespace, the new partitions use it;
+       otherwise the value of <xref linkend="guc-default-tablespace"/> is
+       taken into account, falling back to the database's default tablespace.
      </para>
 
      <para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 265dcfe7fda..fcbfb896607 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -22735,6 +22735,7 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 	Relation	newRel;
 	Oid			newRelId;
 	Oid			existingRelid;
+	Oid			tablespaceId;
 	TupleDesc	descriptor;
 	List	   *colList = NIL;
 	Oid			relamId;
@@ -22786,10 +22787,32 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 				errmsg("cannot create a permanent relation as partition of temporary relation \"%s\"",
 					   RelationGetRelationName(parent_rel)));
 
+	/*
+	 * Select the tablespace for the new partition.  Mirror the logic that
+	 * CREATE TABLE foo PARTITION OF ... uses in DefineRelation: take the
+	 * partitioned parent's explicit tablespace if it has one, otherwise take
+	 * default_tablespace into account, and finally use the database default.
+	 */
+	tablespaceId = parent_relform->reltablespace;
+	if (!OidIsValid(tablespaceId))
+		tablespaceId = GetDefaultTablespace(newPartName->relpersistence, false);
+
+	/* Check permissions except when using database's default */
+	if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
+	{
+		AclResult	aclresult;
+
+		aclresult = object_aclcheck(TableSpaceRelationId, tablespaceId,
+									GetUserId(), ACL_CREATE);
+		if (aclresult != ACLCHECK_OK)
+			aclcheck_error(aclresult, OBJECT_TABLESPACE,
+						   get_tablespace_name(tablespaceId));
+	}
+
 	/* Create the relation. */
 	newRelId = heap_create_with_catalog(newPartName->relname,
 										namespaceId,
-										parent_relform->reltablespace,
+										tablespaceId,
 										InvalidOid,
 										InvalidOid,
 										InvalidOid,
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 4f42afc3dc7..4586b25efe6 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -1114,6 +1114,54 @@ SELECT length(a) FROM t;
   10000
 (1 row)
 
+DROP TABLE t;
+-- Tablespace selection for the new merged partition mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+     spcname      
+------------------
+ regress_tblspace
+(1 row)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partition lands on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+RESET default_tablespace;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+     spcname      
+------------------
+ regress_tblspace
+(1 row)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace and default_tablespace is empty: the
+-- new partition uses the database default (reltablespace = 0).
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged';
+ reltablespace 
+---------------
+             0
+(1 row)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index faaf32ed20a..b1df820a177 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1683,6 +1683,66 @@ SELECT length(a) FROM t;
   10000
 (1 row)
 
+DROP TABLE t;
+-- Tablespace selection for the new partitions mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+ relname |     spcname      
+---------+------------------
+ tp_hi   | regress_tblspace
+ tp_lo   | regress_tblspace
+(2 rows)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partitions land on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+RESET default_tablespace;
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+ relname |     spcname      
+---------+------------------
+ tp_hi   | regress_tblspace
+ tp_lo   | regress_tblspace
+(2 rows)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace and default_tablespace is empty: new
+-- partitions use the database default (reltablespace = 0).
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT relname, reltablespace FROM pg_class
+  WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+ relname | reltablespace 
+---------+---------------
+ tp_hi   |             0
+ tp_lo   |             0
+(2 rows)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 4c8c625f97b..d31ce13dff7 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -798,6 +798,42 @@ SELECT reltoastrelid <> 0 AS has_toast,
 SELECT length(a) FROM t;
 DROP TABLE t;
 
+-- Tablespace selection for the new merged partition mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+DROP TABLE t;
+
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partition lands on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+RESET default_tablespace;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+DROP TABLE t;
+
+-- Parent has no explicit tablespace and default_tablespace is empty: the
+-- new partition uses the database default (reltablespace = 0).
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged';
+DROP TABLE t;
+
 
 RESET search_path;
 
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 9e44aa9caf0..a39be53d3e2 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -1204,6 +1204,51 @@ SELECT relname,
 SELECT length(a) FROM t;
 DROP TABLE t;
 
+-- Tablespace selection for the new partitions mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+DROP TABLE t;
+
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partitions land on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+RESET default_tablespace;
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+DROP TABLE t;
+
+-- Parent has no explicit tablespace and default_tablespace is empty: new
+-- partitions use the database default (reltablespace = 0).
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT relname, reltablespace FROM pg_class
+  WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+DROP TABLE t;
+
 RESET search_path;
 
 --
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-18 15:49                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-21 12:11                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-22 10:44                           ` Pavel Borisov <[email protected]>
  2026-06-24 10:24                             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2026-06-22 10:44 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Alexander!

On Sun, 21 Jun 2026 at 16:11, Alexander Korotkov <[email protected]> wrote:
>
> On Thu, Jun 18, 2026 at 6:49 PM Justin Pryzby <[email protected]> wrote:
> > On Thu, Jun 18, 2026 at 10:31:11AM +0300, Alexander Korotkov wrote:
> > > Pushed with your suggestions accepted.
> >
> > Thanks.  When I went back to test this, I merged ~25 partitions that
> > were all on the same tablespace, but the merged table was created on the
> > default tablespace.
> >
> > I tried again with default_tablespace set, but it was ignored.  I think
> > that's wrong.  It's good to follow the tablespace of the parent table,
> > but if it has no tablespace set, default_tablespace should be obeyed.
> > See surrounding logic in DefineRelation.
> >
> > I see the docs say this:
> > +       <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
> > +       table itself as the template to construct the new partition.
> > +       The new partition will inherit the same table access method, persistence
> > +       type, and tablespace as the partitioned table.
>
> Correct, please see the attached patch, it makes
> createPartitionTable() deal with tablespaces the same was as
> DefineRelation() does.

Thank you for working on this feature!
I've looked into the last v1 patch.

Does it also worth inheriting DefineRelation()'s check and error for
the case if (tablespaceId == GLOBALTABLESPACE_OID)?
A brief look for default_tablespace GUC doesn't reveal a way why it
could not be set to global tablespace in a session.

Best regards,
Pavel Borisov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-18 15:49                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-21 12:11                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-22 10:44                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2026-06-24 10:24                             ` Alexander Korotkov <[email protected]>
  2026-06-24 10:44                               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-24 10:24 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Mon, Jun 22, 2026 at 1:44 PM Pavel Borisov <[email protected]> wrote:
> On Sun, 21 Jun 2026 at 16:11, Alexander Korotkov <[email protected]> wrote:
> >
> > On Thu, Jun 18, 2026 at 6:49 PM Justin Pryzby <[email protected]> wrote:
> > > On Thu, Jun 18, 2026 at 10:31:11AM +0300, Alexander Korotkov wrote:
> > > > Pushed with your suggestions accepted.
> > >
> > > Thanks.  When I went back to test this, I merged ~25 partitions that
> > > were all on the same tablespace, but the merged table was created on the
> > > default tablespace.
> > >
> > > I tried again with default_tablespace set, but it was ignored.  I think
> > > that's wrong.  It's good to follow the tablespace of the parent table,
> > > but if it has no tablespace set, default_tablespace should be obeyed.
> > > See surrounding logic in DefineRelation.
> > >
> > > I see the docs say this:
> > > +       <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
> > > +       table itself as the template to construct the new partition.
> > > +       The new partition will inherit the same table access method, persistence
> > > +       type, and tablespace as the partitioned table.
> >
> > Correct, please see the attached patch, it makes
> > createPartitionTable() deal with tablespaces the same was as
> > DefineRelation() does.
>
> Thank you for working on this feature!
> I've looked into the last v1 patch.
>
> Does it also worth inheriting DefineRelation()'s check and error for
> the case if (tablespaceId == GLOBALTABLESPACE_OID)?
> A brief look for default_tablespace GUC doesn't reveal a way why it
> could not be set to global tablespace in a session.

Thank you for catching this.  I've added this check and corresponding tests.

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v2-0001-Take-into-account-default_tablespace-during-MERGE.patch (16.6K, ../../CAPpHfdtPkk_gQdaKJQgd7Rh6wjqOEu8tGpmni7_1Dgo--rE=oA@mail.gmail.com/2-v2-0001-Take-into-account-default_tablespace-during-MERGE.patch)
  download | inline diff:
From f995f7bc64ceea94dba4b576b4aad2faa8a0834f Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Thu, 18 Jun 2026 21:53:32 +0300
Subject: [PATCH v2] Take into account default_tablespace during MERGE/SPLIT
 PARTITION(S)

createPartitionTable() passed the partitioned parent's reltablespace straight
to heap_create_with_catalog(), bypassing the default_tablespace GUC fallback
that DefineRelation() applies for CREATE TABLE ... PARTITION OF.  When the
parent had no explicit tablespace (reltablespace = 0), the new partition
unconditionally landed in the database default, even if default_tablespace
was set to something else; merging or splitting a set of partitions that all
lived in a non-default tablespace produced a new partition in the database
default.

Mirror DefineRelation()'s logic: take parent's reltablespace if set,
otherwise check GetDefaultTablespace() (which reads default_tablespace
and normalises pg_default / MyDatabaseTableSpace to InvalidOid).  Also
add the CREATE ACL check on the resolved tablespace and the pg_global
rejection, matching DefineRelation()'s behavior.

Update the documentation for MERGE/SPLIT PARTITION to spell out the
tablespace-selection rule explicitly.

Reported-by: Justin Pryzby <[email protected]>
Reviewed-by: Pavel Borisov <[email protected]>
Discussion: https://postgr.es/m/ajQTklv8QArzTp3h%40pryzbyj2023
---
 doc/src/sgml/ref/alter_table.sgml             | 18 +++--
 src/backend/commands/tablecmds.c              | 31 ++++++++-
 src/test/regress/expected/partition_merge.out | 54 +++++++++++++++
 src/test/regress/expected/partition_split.out | 69 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 41 +++++++++++
 src/test/regress/sql/partition_split.sql      | 53 ++++++++++++++
 6 files changed, 261 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index dec34337d1a..36c840f68d8 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1233,8 +1233,13 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      <para>
        <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
        table itself as the template to construct the new partition.
-       The new partition will inherit the same table access method, persistence
-       type, and tablespace as the partitioned table.
+       The new partition inherits the table access method and persistence type
+       of the partitioned table.  Its tablespace is selected as for a
+       <command>CREATE TABLE ... PARTITION OF</command> command issued
+       without a <literal>TABLESPACE</literal> clause: if the partitioned
+       table has an explicit tablespace, the new partition uses it;
+       otherwise the value of <xref linkend="guc-default-tablespace"/> is
+       taken into account, falling back to the database's default tablespace.
 
        Constraints, column defaults, column generation expressions, identity
        columns, indexes, and triggers are copied from the partitioned table to
@@ -1331,8 +1336,13 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      <para>
        <command>ALTER TABLE SPLIT PARTITION</command> uses the partitioned
        table itself as the template to construct new partitions.
-       New partitions will inherit the same table access method, persistence
-       type, and tablespace as the partitioned table.
+       New partitions inherit the table access method and persistence type of
+       the partitioned table.  Their tablespace is selected as for a
+       <command>CREATE TABLE ... PARTITION OF</command> command issued
+       without a <literal>TABLESPACE</literal> clause: if the partitioned
+       table has an explicit tablespace, the new partitions use it;
+       otherwise the value of <xref linkend="guc-default-tablespace"/> is
+       taken into account, falling back to the database's default tablespace.
      </para>
 
      <para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 265dcfe7fda..33e065d61ce 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -22735,6 +22735,7 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 	Relation	newRel;
 	Oid			newRelId;
 	Oid			existingRelid;
+	Oid			tablespaceId;
 	TupleDesc	descriptor;
 	List	   *colList = NIL;
 	Oid			relamId;
@@ -22786,10 +22787,38 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 				errmsg("cannot create a permanent relation as partition of temporary relation \"%s\"",
 					   RelationGetRelationName(parent_rel)));
 
+	/*
+	 * Select the tablespace for the new partition.  Mirror the logic that
+	 * CREATE TABLE foo PARTITION OF ... uses in DefineRelation: take the
+	 * partitioned parent's explicit tablespace if it has one, otherwise take
+	 * default_tablespace into account, and finally use the database default.
+	 */
+	tablespaceId = parent_relform->reltablespace;
+	if (!OidIsValid(tablespaceId))
+		tablespaceId = GetDefaultTablespace(newPartName->relpersistence, false);
+
+	/* Check permissions except when using database's default */
+	if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
+	{
+		AclResult	aclresult;
+
+		aclresult = object_aclcheck(TableSpaceRelationId, tablespaceId,
+									GetUserId(), ACL_CREATE);
+		if (aclresult != ACLCHECK_OK)
+			aclcheck_error(aclresult, OBJECT_TABLESPACE,
+						   get_tablespace_name(tablespaceId));
+	}
+
+	/* In all cases disallow placing user relations in pg_global */
+	if (tablespaceId == GLOBALTABLESPACE_OID)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("only shared relations can be placed in pg_global tablespace")));
+
 	/* Create the relation. */
 	newRelId = heap_create_with_catalog(newPartName->relname,
 										namespaceId,
-										parent_relform->reltablespace,
+										tablespaceId,
 										InvalidOid,
 										InvalidOid,
 										InvalidOid,
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 4f42afc3dc7..d0e1804c24f 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -1114,6 +1114,60 @@ SELECT length(a) FROM t;
   10000
 (1 row)
 
+DROP TABLE t;
+-- Tablespace selection for the new merged partition mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+     spcname      
+------------------
+ regress_tblspace
+(1 row)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partition lands on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+RESET default_tablespace;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+     spcname      
+------------------
+ regress_tblspace
+(1 row)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace and default_tablespace is empty: the
+-- new partition uses the database default (reltablespace = 0).  Also
+-- exercise the pg_global rejection path with default_tablespace pointing
+-- at the shared tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO pg_global;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;	-- fails
+ERROR:  only shared relations can be placed in pg_global tablespace
+RESET default_tablespace;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged';
+ reltablespace 
+---------------
+             0
+(1 row)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index faaf32ed20a..58241a03d3f 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1683,6 +1683,75 @@ SELECT length(a) FROM t;
   10000
 (1 row)
 
+DROP TABLE t;
+-- Tablespace selection for the new partitions mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+ relname |     spcname      
+---------+------------------
+ tp_hi   | regress_tblspace
+ tp_lo   | regress_tblspace
+(2 rows)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partitions land on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+RESET default_tablespace;
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+ relname |     spcname      
+---------+------------------
+ tp_hi   | regress_tblspace
+ tp_lo   | regress_tblspace
+(2 rows)
+
+DROP TABLE t;
+-- Parent has no explicit tablespace and default_tablespace is empty: new
+-- partitions use the database default (reltablespace = 0).  Also exercise
+-- the pg_global rejection path with default_tablespace pointing at the
+-- shared tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO pg_global;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);	-- fails
+ERROR:  only shared relations can be placed in pg_global tablespace
+RESET default_tablespace;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT relname, reltablespace FROM pg_class
+  WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+ relname | reltablespace 
+---------+---------------
+ tp_hi   |             0
+ tp_lo   |             0
+(2 rows)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 4c8c625f97b..d857b4d2fb6 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -798,6 +798,47 @@ SELECT reltoastrelid <> 0 AS has_toast,
 SELECT length(a) FROM t;
 DROP TABLE t;
 
+-- Tablespace selection for the new merged partition mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+DROP TABLE t;
+
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partition lands on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+RESET default_tablespace;
+SELECT spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname = 'tp_merged';
+DROP TABLE t;
+
+-- Parent has no explicit tablespace and default_tablespace is empty: the
+-- new partition uses the database default (reltablespace = 0).  Also
+-- exercise the pg_global rejection path with default_tablespace pointing
+-- at the shared tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_0_5 PARTITION OF t FOR VALUES FROM (0) TO (5);
+CREATE TABLE tp_5_10 PARTITION OF t FOR VALUES FROM (5) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO pg_global;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;	-- fails
+RESET default_tablespace;
+ALTER TABLE t MERGE PARTITIONS (tp_0_5, tp_5_10) INTO tp_merged;
+SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged';
+DROP TABLE t;
+
 
 RESET search_path;
 
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 9e44aa9caf0..e73811670cd 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -1204,6 +1204,59 @@ SELECT relname,
 SELECT length(a) FROM t;
 DROP TABLE t;
 
+-- Tablespace selection for the new partitions mirrors
+-- CREATE TABLE ... PARTITION OF: the partitioned root's explicit
+-- tablespace wins; otherwise default_tablespace applies; otherwise the
+-- database default is used.
+CREATE TABLE t (i int) PARTITION BY RANGE(i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+DROP TABLE t;
+
+-- Parent has no explicit tablespace, but default_tablespace is set: the
+-- new partitions land on default_tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO regress_tblspace;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+RESET default_tablespace;
+SELECT c.relname, s.spcname FROM pg_class c LEFT JOIN pg_tablespace s
+  ON c.reltablespace = s.oid WHERE c.relname IN ('tp_lo', 'tp_hi')
+  ORDER BY c.relname;
+DROP TABLE t;
+
+-- Parent has no explicit tablespace and default_tablespace is empty: new
+-- partitions use the database default (reltablespace = 0).  Also exercise
+-- the pg_global rejection path with default_tablespace pointing at the
+-- shared tablespace.
+CREATE TABLE t (i int) PARTITION BY RANGE(i);
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (0) TO (10);
+INSERT INTO t SELECT generate_series(0, 9);
+SET default_tablespace TO pg_global;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);	-- fails
+RESET default_tablespace;
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (0) TO (5),
+    PARTITION tp_hi FOR VALUES FROM (5) TO (10)
+);
+SELECT relname, reltablespace FROM pg_class
+  WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+DROP TABLE t;
+
 RESET search_path;
 
 --
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-18 15:49                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-21 12:11                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-22 10:44                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-24 10:24                             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-24 10:44                               ` Pavel Borisov <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Pavel Borisov @ 2026-06-24 10:44 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Alexander!

On Wed, 24 Jun 2026 at 14:25, Alexander Korotkov <[email protected]> wrote:
>
> On Mon, Jun 22, 2026 at 1:44 PM Pavel Borisov <[email protected]> wrote:
> > On Sun, 21 Jun 2026 at 16:11, Alexander Korotkov <[email protected]> wrote:
> > >
> > > On Thu, Jun 18, 2026 at 6:49 PM Justin Pryzby <[email protected]> wrote:
> > > > On Thu, Jun 18, 2026 at 10:31:11AM +0300, Alexander Korotkov wrote:
> > > > > Pushed with your suggestions accepted.
> > > >
> > > > Thanks.  When I went back to test this, I merged ~25 partitions that
> > > > were all on the same tablespace, but the merged table was created on the
> > > > default tablespace.
> > > >
> > > > I tried again with default_tablespace set, but it was ignored.  I think
> > > > that's wrong.  It's good to follow the tablespace of the parent table,
> > > > but if it has no tablespace set, default_tablespace should be obeyed.
> > > > See surrounding logic in DefineRelation.
> > > >
> > > > I see the docs say this:
> > > > +       <command>ALTER TABLE MERGE PARTITION</command> uses the partitioned
> > > > +       table itself as the template to construct the new partition.
> > > > +       The new partition will inherit the same table access method, persistence
> > > > +       type, and tablespace as the partitioned table.
> > >
> > > Correct, please see the attached patch, it makes
> > > createPartitionTable() deal with tablespaces the same was as
> > > DefineRelation() does.
> >
> > Thank you for working on this feature!
> > I've looked into the last v1 patch.
> >
> > Does it also worth inheriting DefineRelation()'s check and error for
> > the case if (tablespaceId == GLOBALTABLESPACE_OID)?
> > A brief look for default_tablespace GUC doesn't reveal a way why it
> > could not be set to global tablespace in a session.
>
> Thank you for catching this.  I've added this check and corresponding tests.

Looked at the patch v2.
I think in test we'd better split comments for the failing block (SET
default_tablespace TO pg_global;) and for the successful block (after
RESET default_tablespace;). Otherwise, the comment paragraph looks a
little hard to read to me:
+-- Parent has no explicit tablespace and default_tablespace is empty: the
+-- new partition uses the database default (reltablespace = 0).  Also
+-- exercise the pg_global rejection path with default_tablespace pointing
+-- at the shared tablespace.

Overall the patch looks good to me.

Regards,
Pavel Borisov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-26 13:40                       ` Justin Pryzby <[email protected]>
  2026-06-26 13:47                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Justin Pryzby @ 2026-06-26 13:40 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Thanks for addressing the tablespace issue.

One more thing -- merging non-contiguous tables is currently refused:

ts=# CREATE TABLE a(t timestamptz) PARTITION BY RANGE (t);
ts=# CREATE TABLE a_20260101 PARTITION OF a FOR VALUES FROM ('20260101')TO('20260102');
ts=# CREATE TABLE a_20260126 PARTITION OF a FOR VALUES FROM ('20260126')TO('20260127');
ts=# CREATE TABLE a_20260127 PARTITION OF a FOR VALUES FROM ('20260127')TO('20260128');
ts=# CREATE TABLE a_20260128 PARTITION OF a FOR VALUES FROM ('20260128')TO('20260129');

ts=# ALTER TABLE a MERGE PARTITIONS (a_20260101,a_20260126,a_20260127,a_20260128) INTO a_202601;
ERROR:  cannot merge partition "a_20260126" together with partition "a_20260101"
DETAIL:  The lower bound of partition "a_20260126" is not equal to the upper bound of partition "a_20260101".
HINT:  ALTER TABLE ... MERGE PARTITIONS requires the partition bounds to be adjacent.

The goal seems to be to avoid overlapping partition constaints.

If there's a default partition, that also avoids the possibility of
needing to move *some* (but not all) tuples from it into the merged
partition.

I think there should be separate logic depending on the existance of a
default partition:

- The existing logic seems to correctly handle the case of a default
  partition, which is being merged (gap is allowed).

- The case of a default partition which is not being merged is also
  handled: no gap is allowed.  Actually, this case was probably the
  motivation behind checking that the partitions are adjacent.

- If there's *no* default partition, then I think the check should be
  relaxed; it's sufficient to verify that the bounds of the merged
  partition do not overlap with any partition which is not being merged;

-- 
Justin





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
@ 2026-06-26 13:47                         ` Pavel Borisov <[email protected]>
  2026-06-26 15:20                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2026-06-26 13:47 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

Hi, Justin!

On Fri, 26 Jun 2026 at 17:40, Justin Pryzby <[email protected]> wrote:
>
> Thanks for addressing the tablespace issue.
>
> One more thing -- merging non-contiguous tables is currently refused:
>
> ts=# CREATE TABLE a(t timestamptz) PARTITION BY RANGE (t);
> ts=# CREATE TABLE a_20260101 PARTITION OF a FOR VALUES FROM ('20260101')TO('20260102');
> ts=# CREATE TABLE a_20260126 PARTITION OF a FOR VALUES FROM ('20260126')TO('20260127');
> ts=# CREATE TABLE a_20260127 PARTITION OF a FOR VALUES FROM ('20260127')TO('20260128');
> ts=# CREATE TABLE a_20260128 PARTITION OF a FOR VALUES FROM ('20260128')TO('20260129');
>
> ts=# ALTER TABLE a MERGE PARTITIONS (a_20260101,a_20260126,a_20260127,a_20260128) INTO a_202601;
> ERROR:  cannot merge partition "a_20260126" together with partition "a_20260101"
> DETAIL:  The lower bound of partition "a_20260126" is not equal to the upper bound of partition "a_20260101".
> HINT:  ALTER TABLE ... MERGE PARTITIONS requires the partition bounds to be adjacent.
>
> The goal seems to be to avoid overlapping partition constaints.
>
> If there's a default partition, that also avoids the possibility of
> needing to move *some* (but not all) tuples from it into the merged
> partition.
>
> I think there should be separate logic depending on the existance of a
> default partition:
>
> - The existing logic seems to correctly handle the case of a default
>   partition, which is being merged (gap is allowed).
>
> - The case of a default partition which is not being merged is also
>   handled: no gap is allowed.  Actually, this case was probably the
>   motivation behind checking that the partitions are adjacent.
>
> - If there's *no* default partition, then I think the check should be
>   relaxed; it's sufficient to verify that the bounds of the merged
>   partition do not overlap with any partition which is not being merged;

Thank you for reporting on this feature!
Yes, I think main motivation for not allowing a partition bounds gaps for
merge was the complication in case a DEFAULT partition exists.
Probably, this could be relaxed when there is no DEFAULT partition.

Regards,
Pavel Borisov
Supabase





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-26 13:47                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2026-06-26 15:20                           ` Dmitry Koval <[email protected]>
  2026-06-26 19:50                             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Dmitry Koval @ 2026-06-26 15:20 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; +Cc: [email protected]

Hi!

If we exclude case with DEFAULT partition, MERGE PARTITIONS command was
intended to be used when an incorrect table partitioning was chosen.
For example, a table was initially partitioned by month, but later we
needed to change table partitioning by quarter. In this case, MERGE
PARTITIONS command should merge several adjacent partitions into one.
Current checks are made for this case.

 >- If there's *no* default partition, then I think the check should be
 >relaxed; it's sufficient to verify that the bounds of the merged
 >partition do not overlap with any partition which is not being merged;

It's probably possible to do this. But in this case, the command will
not exactly be "MERGE PARTITIONS" ("MERGE PARTITIONS & EXPAND"?).

-- 
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-26 13:47                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-26 15:20                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
@ 2026-06-26 19:50                             ` Alexander Korotkov <[email protected]>
  2026-06-30 11:06                               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-26 19:50 UTC (permalink / raw)
  To: Dmitry Koval <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; [email protected]

On Fri, Jun 26, 2026 at 6:20 PM Dmitry Koval <[email protected]> wrote:
> If we exclude case with DEFAULT partition, MERGE PARTITIONS command was
> intended to be used when an incorrect table partitioning was chosen.
> For example, a table was initially partitioned by month, but later we
> needed to change table partitioning by quarter. In this case, MERGE
> PARTITIONS command should merge several adjacent partitions into one.
> Current checks are made for this case.
>
>  >- If there's *no* default partition, then I think the check should be
>  >relaxed; it's sufficient to verify that the bounds of the merged
>  >partition do not overlap with any partition which is not being merged;
>
> It's probably possible to do this. But in this case, the command will
> not exactly be "MERGE PARTITIONS" ("MERGE PARTITIONS & EXPAND"?).

+1,
We can implement a support for this for 20.  For 19, I propose to
state the restriction more explicit in the docs.  See the attached
patch.

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v1-0001-doc-clarify-MERGE-PARTITIONS-adjacency-requiremen.patch (1.6K, ../../CAPpHfdsao-VXZYN8-vRm543EFUE7=4tq+hcTh5iEc6ikqWq_cQ@mail.gmail.com/2-v1-0001-doc-clarify-MERGE-PARTITIONS-adjacency-requiremen.patch)
  download | inline diff:
From b60a0d867443af7ab45ec70de5d65df8f93ab90f Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 26 Jun 2026 22:44:01 +0300
Subject: [PATCH v1] doc: clarify MERGE PARTITIONS adjacency requirement

The existing description says the ranges of merged range-partitions
"must be adjacent in order to be merged" only under the heading
"If the DEFAULT partition is not in the list of merged partitions",
which can be misread as a restriction tied to the presence of a
default partition.  In fact, merging non-adjacent ranges is rejected
regardless of whether the partitioned table has a default partition;
spell that out explicitly.

Reported-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/aj6BPoziSb-F8aJz%40pryzbyj2023
---
 doc/src/sgml/ref/alter_table.sgml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 67a05593140..0df7396f268 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1196,7 +1196,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
        <listitem>
         <para>
          For range-partitioned tables, the ranges of merged partitions
-         must be adjacent in order to be merged.
+         must be adjacent in order to be merged; this applies even if
+         the partitioned table has no default partition.
          The partition bounds of merged partitions are combined to form the new partition bound for
          <replaceable class="parameter">partition_name</replaceable>.
         </para>
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-26 13:47                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-26 15:20                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
  2026-06-26 19:50                             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-30 11:06                               ` Alexander Korotkov <[email protected]>
  2026-06-30 11:25                                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-30 11:06 UTC (permalink / raw)
  To: Dmitry Koval <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; [email protected]

On Fri, Jun 26, 2026 at 10:50 PM Alexander Korotkov
<[email protected]> wrote:
> On Fri, Jun 26, 2026 at 6:20 PM Dmitry Koval <[email protected]> wrote:
> > If we exclude case with DEFAULT partition, MERGE PARTITIONS command was
> > intended to be used when an incorrect table partitioning was chosen.
> > For example, a table was initially partitioned by month, but later we
> > needed to change table partitioning by quarter. In this case, MERGE
> > PARTITIONS command should merge several adjacent partitions into one.
> > Current checks are made for this case.
> >
> >  >- If there's *no* default partition, then I think the check should be
> >  >relaxed; it's sufficient to verify that the bounds of the merged
> >  >partition do not overlap with any partition which is not being merged;
> >
> > It's probably possible to do this. But in this case, the command will
> > not exactly be "MERGE PARTITIONS" ("MERGE PARTITIONS & EXPAND"?).
>
> +1,
> We can implement a support for this for 20.  For 19, I propose to
> state the restriction more explicit in the docs.  See the attached
> patch.

I'm going to push the docs patch if no objections.

------
Regards,
Alexander Korotkov
Supabase





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-26 13:47                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-26 15:20                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
  2026-06-26 19:50                             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-30 11:06                               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-30 11:25                                 ` Pavel Borisov <[email protected]>
  2026-06-30 19:38                                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Pavel Borisov @ 2026-06-30 11:25 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Justin Pryzby <[email protected]>; [email protected]

Hi, Alexander!

On Tue, 30 Jun 2026 at 15:07, Alexander Korotkov <[email protected]> wrote:
>
> On Fri, Jun 26, 2026 at 10:50 PM Alexander Korotkov
> <[email protected]> wrote:
> > On Fri, Jun 26, 2026 at 6:20 PM Dmitry Koval <[email protected]> wrote:
> > > If we exclude case with DEFAULT partition, MERGE PARTITIONS command was
> > > intended to be used when an incorrect table partitioning was chosen.
> > > For example, a table was initially partitioned by month, but later we
> > > needed to change table partitioning by quarter. In this case, MERGE
> > > PARTITIONS command should merge several adjacent partitions into one.
> > > Current checks are made for this case.
> > >
> > >  >- If there's *no* default partition, then I think the check should be
> > >  >relaxed; it's sufficient to verify that the bounds of the merged
> > >  >partition do not overlap with any partition which is not being merged;
> > >
> > > It's probably possible to do this. But in this case, the command will
> > > not exactly be "MERGE PARTITIONS" ("MERGE PARTITIONS & EXPAND"?).
> >
> > +1,
> > We can implement a support for this for 20.  For 19, I propose to
> > state the restriction more explicit in the docs.  See the attached
> > patch.
>
> I'm going to push the docs patch if no objections.
I looked at the doc patch. IMO "in order to be merged" is redundant.
I'd propose following:
"For range-partitioned tables, the ranges of merged partitions must be
adjacent. This applies even if the partitioned table has no default
partition."

Regards,
Pavel Borisov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:56               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-16 12:22                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 13:01                   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-18 07:31                     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-26 13:40                       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-26 13:47                         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-26 15:20                           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
  2026-06-26 19:50                             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-30 11:06                               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-30 11:25                                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
@ 2026-06-30 19:38                                   ` Alexander Korotkov <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-30 19:38 UTC (permalink / raw)
  To: Pavel Borisov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Justin Pryzby <[email protected]>; [email protected]

Hi, Pavel!

On Tue, Jun 30, 2026 at 2:25 PM Pavel Borisov <[email protected]> wrote:
> On Tue, 30 Jun 2026 at 15:07, Alexander Korotkov <[email protected]> wrote:
> > On Fri, Jun 26, 2026 at 10:50 PM Alexander Korotkov
> > <[email protected]> wrote:
> > > On Fri, Jun 26, 2026 at 6:20 PM Dmitry Koval <[email protected]> wrote:
> > > > If we exclude case with DEFAULT partition, MERGE PARTITIONS command was
> > > > intended to be used when an incorrect table partitioning was chosen.
> > > > For example, a table was initially partitioned by month, but later we
> > > > needed to change table partitioning by quarter. In this case, MERGE
> > > > PARTITIONS command should merge several adjacent partitions into one.
> > > > Current checks are made for this case.
> > > >
> > > >  >- If there's *no* default partition, then I think the check should be
> > > >  >relaxed; it's sufficient to verify that the bounds of the merged
> > > >  >partition do not overlap with any partition which is not being merged;
> > > >
> > > > It's probably possible to do this. But in this case, the command will
> > > > not exactly be "MERGE PARTITIONS" ("MERGE PARTITIONS & EXPAND"?).
> > >
> > > +1,
> > > We can implement a support for this for 20.  For 19, I propose to
> > > state the restriction more explicit in the docs.  See the attached
> > > patch.
> >
> > I'm going to push the docs patch if no objections.
> I looked at the doc patch. IMO "in order to be merged" is redundant.
> I'd propose following:
> "For range-partitioned tables, the ranges of merged partitions must be
> adjacent. This applies even if the partitioned table has no default
> partition."

I've included your suggestion.
Pushed and and backpatched to 19.

------
Regards,
Alexander Korotkov
Supabase






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
@ 2026-06-16 08:08               ` jian he <[email protected]>
  2026-06-16 12:19                 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: jian he @ 2026-06-16 08:08 UTC (permalink / raw)
  To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Tue, Jun 16, 2026 at 5:37 AM Alexander Korotkov <[email protected]> wrote:
>
> On Tue, Jun 16, 2026 at 12:36 AM Pavel Borisov <[email protected]> wrote:
> > On Tue, 16 Jun 2026 at 00:50, Alexander Korotkov <[email protected]> wrote:
> > >
> > > On Mon, Jun 15, 2026 at 2:07 PM Justin Pryzby <[email protected]> wrote:
> > > >
> > > > I hit an error when I tried this patch.
> > > >
> > > > CREATE TABLE a (a text) PARTITION BY RANGE(a);
> > > > CREATE TABLE a1 PARTITION OF a DEFAULT;
> > > > INSERT INTO a SELECT repeat('1', 9999999);
> > > > CREATE TABLE a2 PARTITION OF a FOR VALUES FROM (2)TO(3);
> > > > ALTER TABLE a MERGE PARTITIONS (a1, a2) INTO a1;
> > > > ERROR:  row is too big: size 39264, maximum size 8160
> > >
> > > Thank you for your report.  It appears that createPartitionTable()
> > > misses NewRelationCreateToastTable() call (for instance, DefineTable()
> > > has this call).  The attached patch implements fix and has tests.  I'm
> > > going to push it if no objections.
> >
> > It looks like the patch is missing in the last message.
>

src5=# explain (analyze, timing, costs off) SELECT repeat('1', 9999999);
                     QUERY PLAN
-----------------------------------------------------
 Result (actual time=0.002..0.003 rows=1.00 loops=1)
 Planning Time: 1454.745 ms
 Execution Time: 0.035 ms
(3 rows)

Time: 1458.413 ms (00:01.458)

The timing above is from an Assert-enabled build.
Could we optimize these tests to make them less expensive to run?



--
jian
https://www.enterprisedb.com/






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-08 21:51 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2024-05-14 14:49   ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2024-05-17 10:05     ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 11:07       ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
  2026-06-15 20:50         ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-15 21:36           ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
  2026-06-15 21:36             ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
  2026-06-16 08:08               ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands jian he <[email protected]>
@ 2026-06-16 12:19                 ` Alexander Korotkov <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Alexander Korotkov @ 2026-06-16 12:19 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Justin Pryzby <[email protected]>; Dmitry Koval <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Alvaro Herrera <[email protected]>

On Tue, Jun 16, 2026 at 11:09 AM jian he <[email protected]> wrote:
>
> On Tue, Jun 16, 2026 at 5:37 AM Alexander Korotkov <[email protected]> wrote:
> >
> > On Tue, Jun 16, 2026 at 12:36 AM Pavel Borisov <[email protected]> wrote:
> > > On Tue, 16 Jun 2026 at 00:50, Alexander Korotkov <[email protected]> wrote:
> > > >
> > > > On Mon, Jun 15, 2026 at 2:07 PM Justin Pryzby <[email protected]> wrote:
> > > > >
> > > > > I hit an error when I tried this patch.
> > > > >
> > > > > CREATE TABLE a (a text) PARTITION BY RANGE(a);
> > > > > CREATE TABLE a1 PARTITION OF a DEFAULT;
> > > > > INSERT INTO a SELECT repeat('1', 9999999);
> > > > > CREATE TABLE a2 PARTITION OF a FOR VALUES FROM (2)TO(3);
> > > > > ALTER TABLE a MERGE PARTITIONS (a1, a2) INTO a1;
> > > > > ERROR:  row is too big: size 39264, maximum size 8160
> > > >
> > > > Thank you for your report.  It appears that createPartitionTable()
> > > > misses NewRelationCreateToastTable() call (for instance, DefineTable()
> > > > has this call).  The attached patch implements fix and has tests.  I'm
> > > > going to push it if no objections.
> > >
> > > It looks like the patch is missing in the last message.
> >
>
> src5=# explain (analyze, timing, costs off) SELECT repeat('1', 9999999);
>                      QUERY PLAN
> -----------------------------------------------------
>  Result (actual time=0.002..0.003 rows=1.00 loops=1)
>  Planning Time: 1454.745 ms
>  Execution Time: 0.035 ms
> (3 rows)
>
> Time: 1458.413 ms (00:01.458)
>
> The timing above is from an Assert-enabled build.
> Could we optimize these tests to make them less expensive to run?

Yes, we can.  Here is the second version of this patch.  Now it uses
just 10'000 bytes value with external storage.

------
Regards,
Alexander Korotkov
Supabase


Attachments:

  [application/octet-stream] v2-0001-Create-TOAST-table-for-partitions-made-by-MERGE-S.patch (7.8K, ../../CAPpHfdv8N7cRu9BxN=_9ZewdoseUs7_i_RfsHdkp32kY3zSHVQ@mail.gmail.com/2-v2-0001-Create-TOAST-table-for-partitions-made-by-MERGE-S.patch)
  download | inline diff:
From a33137fe9c853f90f48a643963b39ec92017307b Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 15 Jun 2026 15:05:23 +0300
Subject: [PATCH v2] Create TOAST table for partitions made by MERGE/SPLIT
 PARTITION

ALTER TABLE ... MERGE PARTITIONS / SPLIT PARTITION builds a new
partition via createPartitionTable(), but never gave it a TOAST table.
When the source rows carried out-of-line varlena values, the move
into the new partition entered heap_toast_insert_or_update() with
reltoastrelid = InvalidOid: the externalization step is skipped, the
value falls back to inline storage and heap_insert() fails with
"row is too big" error.  Also, TOAST table is needed if the new partition
receives out-of-line varlena values after the DDL operation is complete.

Call NewRelationCreateToastTable() right after the new partition is
created in createPartitionTable(), mirroring what DefineRelation()
does for regular CREATE TABLE.  NewRelationCreateToastTable() decides
on its own whether a TOAST table is actually required, so partitions
with no toast-eligible columns are unaffected.

Reported-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/ai_c4-v8iLA2kXFV%40pryzbyj2023
Reviewed-by: Pavel Borisov <[email protected]>
Reviewed-by: Jian He <[email protected]>
---
 src/backend/commands/tablecmds.c              |  9 ++++++
 src/test/regress/expected/partition_merge.out | 26 ++++++++++++++++
 src/test/regress/expected/partition_split.out | 30 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 17 +++++++++++
 src/test/regress/sql/partition_split.sql      | 19 ++++++++++++
 5 files changed, 101 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 38f9ffcd04f..265dcfe7fda 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -22815,6 +22815,15 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 	 */
 	CommandCounterIncrement();
 
+	/*
+	 * Create a TOAST table if the table needs one.  MERGE/SPLIT PARTITION
+	 * moves rows from existing partition(s) into new partition(s), which may
+	 * carry out-of-line varlena values that the new relation must be able to
+	 * store.  Also, the new partition must be able to receive out-of-line
+	 * varlena values after the DDL operation is complete.
+	 */
+	NewRelationCreateToastTable(newRelId, (Datum) 0);
+
 	/*
 	 * Open the new partition with no lock, because we already have an
 	 * AccessExclusiveLock placed there after creation.
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index d3818f1bf9b..9ee548cbcc8 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -1088,6 +1088,32 @@ SELECT count(*) FROM t WHERE i = 15 AND g IN (SELECT g + 10 FROM t WHERE i = 5);
      1
 (1 row)
 
+DROP TABLE t;
+-- Merged partitions need their own TOAST table; otherwise an out-of-line
+-- varlena value carried over from one of the merging partitions has
+-- nowhere to be re-stored.  SET STORAGE EXTERNAL forces externalization
+-- for any value over the TOAST threshold, so a short string suffices to
+-- exercise the toast-table dependency.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+ALTER TABLE t ALTER COLUMN a SET STORAGE EXTERNAL;
+CREATE TABLE tp_def PARTITION OF t DEFAULT;
+CREATE TABLE tp_2_3 PARTITION OF t FOR VALUES FROM ('2') TO ('3');
+INSERT INTO t SELECT repeat('1', 10000);
+ALTER TABLE t MERGE PARTITIONS (tp_def, tp_2_3) INTO tp_merged;
+SELECT reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname = 'tp_merged';
+ has_toast | toast_used 
+-----------+------------
+ t         | t
+(1 row)
+
+SELECT length(a) FROM t;
+ length 
+--------
+  10000
+(1 row)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index ff6027af658..9a23657e750 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1653,6 +1653,36 @@ SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i =
      0
 (1 row)
 
+DROP TABLE t;
+-- Each new partition produced by SPLIT must get its own TOAST table so
+-- that out-of-line varlena attributes coming from the source partition
+-- can be stored.  SET STORAGE EXTERNAL forces externalization for any
+-- value over the TOAST threshold, so a short string suffices to exercise
+-- the toast-table dependency.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+ALTER TABLE t ALTER COLUMN a SET STORAGE EXTERNAL;
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
+INSERT INTO t SELECT repeat('1', 10000);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (MINVALUE) TO ('2'),
+    PARTITION tp_hi FOR VALUES FROM ('2') TO (MAXVALUE)
+);
+SELECT relname,
+       reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+ relname | has_toast | toast_used 
+---------+-----------+------------
+ tp_hi   | t         | f
+ tp_lo   | t         | t
+(2 rows)
+
+SELECT length(a) FROM t;
+ length 
+--------
+  10000
+(1 row)
+
 DROP TABLE t;
 RESET search_path;
 --
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 1e14ed40f5c..2721e648228 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -781,6 +781,23 @@ SELECT count(*) FROM t WHERE i = 15 AND g IN (SELECT g + 10 FROM t WHERE i = 5);
 
 DROP TABLE t;
 
+-- Merged partitions need their own TOAST table; otherwise an out-of-line
+-- varlena value carried over from one of the merging partitions has
+-- nowhere to be re-stored.  SET STORAGE EXTERNAL forces externalization
+-- for any value over the TOAST threshold, so a short string suffices to
+-- exercise the toast-table dependency.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+ALTER TABLE t ALTER COLUMN a SET STORAGE EXTERNAL;
+CREATE TABLE tp_def PARTITION OF t DEFAULT;
+CREATE TABLE tp_2_3 PARTITION OF t FOR VALUES FROM ('2') TO ('3');
+INSERT INTO t SELECT repeat('1', 10000);
+ALTER TABLE t MERGE PARTITIONS (tp_def, tp_2_3) INTO tp_merged;
+SELECT reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname = 'tp_merged';
+SELECT length(a) FROM t;
+DROP TABLE t;
+
 
 RESET search_path;
 
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 05de24152d1..48d96e3311e 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -1184,6 +1184,25 @@ SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i =
 
 DROP TABLE t;
 
+-- Each new partition produced by SPLIT must get its own TOAST table so
+-- that out-of-line varlena attributes coming from the source partition
+-- can be stored.  SET STORAGE EXTERNAL forces externalization for any
+-- value over the TOAST threshold, so a short string suffices to exercise
+-- the toast-table dependency.
+CREATE TABLE t (a text) PARTITION BY RANGE(a);
+ALTER TABLE t ALTER COLUMN a SET STORAGE EXTERNAL;
+CREATE TABLE tp_all PARTITION OF t FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
+INSERT INTO t SELECT repeat('1', 10000);
+ALTER TABLE t SPLIT PARTITION tp_all INTO (
+    PARTITION tp_lo FOR VALUES FROM (MINVALUE) TO ('2'),
+    PARTITION tp_hi FOR VALUES FROM ('2') TO (MAXVALUE)
+);
+SELECT relname,
+       reltoastrelid <> 0 AS has_toast,
+       pg_relation_size(reltoastrelid) > 0 AS toast_used
+  FROM pg_class WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname;
+SELECT length(a) FROM t;
+DROP TABLE t;
 
 RESET search_path;
 
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v20 5/8] Row pattern recognition patch (executor).
@ 2024-05-24 02:26 Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Tatsuo Ishii @ 2024-05-24 02:26 UTC (permalink / raw)

---
 src/backend/executor/nodeWindowAgg.c | 1610 +++++++++++++++++++++++++-
 src/backend/utils/adt/windowfuncs.c  |   37 +-
 src/include/catalog/pg_proc.dat      |    6 +
 src/include/nodes/execnodes.h        |   30 +
 4 files changed, 1671 insertions(+), 12 deletions(-)

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 3221fa1522..140bb3941e 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -36,6 +36,7 @@
 #include "access/htup_details.h"
 #include "catalog/objectaccess.h"
 #include "catalog/pg_aggregate.h"
+#include "catalog/pg_collation_d.h"
 #include "catalog/pg_proc.h"
 #include "executor/executor.h"
 #include "executor/nodeWindowAgg.h"
@@ -48,6 +49,7 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
+#include "utils/fmgroids.h"
 #include "utils/expandeddatum.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -159,6 +161,43 @@ typedef struct WindowStatePerAggData
 	bool		restart;		/* need to restart this agg in this cycle? */
 } WindowStatePerAggData;
 
+/*
+ * Set of StringInfo. Used in RPR.
+ */
+typedef struct StringSet
+{
+	StringInfo *str_set;
+	Size		set_size;		/* current array allocation size in number of
+								 * items */
+	int			set_index;		/* current used size */
+}			StringSet;
+
+/*
+ * Allowed subsequent PATTERN variables positions.
+ * Used in RPR.
+ *
+ * pos represents the pattern variable defined order in DEFINE caluase.  For
+ * example. "DEFINE START..., UP..., DOWN ..." and "PATTERN START UP DOWN UP"
+ * will create:
+ * VariablePos[0].pos[0] = 0;		START
+ * VariablePos[1].pos[0] = 1;		UP
+ * VariablePos[1].pos[1] = 3;		UP
+ * VariablePos[2].pos[0] = 2;		DOWN
+ *
+ * Note that UP has two pos because UP appears in PATTERN twice.
+ *
+ * By using this strucrture, we can know which pattern variable can be followed
+ * by which pattern variable(s). For example, START can be followed by UP and
+ * DOWN since START's pos is 0, and UP's pos is 1 or 3, DOWN's pos is 2.
+ * DOWN can be followed by UP since UP's pos is either 1 or 3.
+ *
+ */
+#define NUM_ALPHABETS	26		/* we allow [a-z] variable initials */
+typedef struct VariablePos
+{
+	int			pos[NUM_ALPHABETS]; /* postion(s) in PATTERN */
+}			VariablePos;
+
 static void initialize_windowaggregate(WindowAggState *winstate,
 									   WindowStatePerFunc perfuncstate,
 									   WindowStatePerAgg peraggstate);
@@ -184,6 +223,7 @@ static void release_partition(WindowAggState *winstate);
 
 static int	row_is_in_frame(WindowAggState *winstate, int64 pos,
 							TupleTableSlot *slot);
+
 static void update_frameheadpos(WindowAggState *winstate);
 static void update_frametailpos(WindowAggState *winstate);
 static void update_grouptailpos(WindowAggState *winstate);
@@ -195,9 +235,48 @@ static Datum GetAggInitVal(Datum textInitVal, Oid transtype);
 
 static bool are_peers(WindowAggState *winstate, TupleTableSlot *slot1,
 					  TupleTableSlot *slot2);
+
+static int	WinGetSlotInFrame(WindowObject winobj, TupleTableSlot *slot,
+							  int relpos, int seektype, bool set_mark,
+							  bool *isnull, bool *isout);
 static bool window_gettupleslot(WindowObject winobj, int64 pos,
 								TupleTableSlot *slot);
 
+static void attno_map(Node *node);
+static bool attno_map_walker(Node *node, void *context);
+static int	row_is_in_reduced_frame(WindowObject winobj, int64 pos);
+static bool rpr_is_defined(WindowAggState *winstate);
+
+static void create_reduced_frame_map(WindowAggState *winstate);
+static int	get_reduced_frame_map(WindowAggState *winstate, int64 pos);
+static void register_reduced_frame_map(WindowAggState *winstate, int64 pos,
+									   int val);
+static void clear_reduced_frame_map(WindowAggState *winstate);
+static void update_reduced_frame(WindowObject winobj, int64 pos);
+
+static int64 evaluate_pattern(WindowObject winobj, int64 current_pos,
+							  char *vname, StringInfo encoded_str, bool *result);
+
+static bool get_slots(WindowObject winobj, int64 current_pos);
+
+static int	search_str_set(char *pattern, StringSet * str_set,
+						   VariablePos * variable_pos);
+static char pattern_initial(WindowAggState *winstate, char *vname);
+static int	do_pattern_match(char *pattern, char *encoded_str);
+
+static StringSet * string_set_init(void);
+static void string_set_add(StringSet * string_set, StringInfo str);
+static StringInfo string_set_get(StringSet * string_set, int index);
+static int	string_set_get_size(StringSet * string_set);
+static void string_set_discard(StringSet * string_set);
+static VariablePos * variable_pos_init(void);
+static void variable_pos_register(VariablePos * variable_pos, char initial,
+								  int pos);
+static bool variable_pos_compare(VariablePos * variable_pos,
+								 char initial1, char initial2);
+static int	variable_pos_fetch(VariablePos * variable_pos, char initial,
+							   int index);
+static void variable_pos_discard(VariablePos * variable_pos);
 
 /*
  * initialize_windowaggregate
@@ -774,10 +853,12 @@ eval_windowaggregates(WindowAggState *winstate)
 	 *	   transition function, or
 	 *	 - we have an EXCLUSION clause, or
 	 *	 - if the new frame doesn't overlap the old one
+	 *   - if RPR is enabled
 	 *
 	 * Note that we don't strictly need to restart in the last case, but if
 	 * we're going to remove all rows from the aggregation anyway, a restart
 	 * surely is faster.
+	 *     we restart aggregation too.
 	 *----------
 	 */
 	numaggs_restart = 0;
@@ -788,7 +869,8 @@ eval_windowaggregates(WindowAggState *winstate)
 			(winstate->aggregatedbase != winstate->frameheadpos &&
 			 !OidIsValid(peraggstate->invtransfn_oid)) ||
 			(winstate->frameOptions & FRAMEOPTION_EXCLUSION) ||
-			winstate->aggregatedupto <= winstate->frameheadpos)
+			winstate->aggregatedupto <= winstate->frameheadpos ||
+			rpr_is_defined(winstate))
 		{
 			peraggstate->restart = true;
 			numaggs_restart++;
@@ -862,7 +944,22 @@ eval_windowaggregates(WindowAggState *winstate)
 	 * head, so that tuplestore can discard unnecessary rows.
 	 */
 	if (agg_winobj->markptr >= 0)
-		WinSetMarkPosition(agg_winobj, winstate->frameheadpos);
+	{
+		int64		markpos = winstate->frameheadpos;
+
+		if (rpr_is_defined(winstate))
+		{
+			/*
+			 * If RPR is used, it is possible PREV wants to look at the
+			 * previous row.  So the mark pos should be frameheadpos - 1
+			 * unless it is below 0.
+			 */
+			markpos -= 1;
+			if (markpos < 0)
+				markpos = 0;
+		}
+		WinSetMarkPosition(agg_winobj, markpos);
+	}
 
 	/*
 	 * Now restart the aggregates that require it.
@@ -917,6 +1014,14 @@ eval_windowaggregates(WindowAggState *winstate)
 	{
 		winstate->aggregatedupto = winstate->frameheadpos;
 		ExecClearTuple(agg_row_slot);
+
+		/*
+		 * If RPR is defined, we do not use aggregatedupto_nonrestarted.  To
+		 * avoid assertion failure below, we reset aggregatedupto_nonrestarted
+		 * to frameheadpos.
+		 */
+		if (rpr_is_defined(winstate))
+			aggregatedupto_nonrestarted = winstate->frameheadpos;
 	}
 
 	/*
@@ -930,6 +1035,12 @@ eval_windowaggregates(WindowAggState *winstate)
 	{
 		int			ret;
 
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "===== loop in frame starts: aggregatedupto: " INT64_FORMAT " aggregatedbase: " INT64_FORMAT,
+			 winstate->aggregatedupto,
+			 winstate->aggregatedbase);
+#endif
+
 		/* Fetch next row if we didn't already */
 		if (TupIsNull(agg_row_slot))
 		{
@@ -945,9 +1056,52 @@ eval_windowaggregates(WindowAggState *winstate)
 		ret = row_is_in_frame(winstate, winstate->aggregatedupto, agg_row_slot);
 		if (ret < 0)
 			break;
+
 		if (ret == 0)
 			goto next_tuple;
 
+		if (rpr_is_defined(winstate))
+		{
+#ifdef RPR_DEBUG
+			elog(DEBUG1, "reduced_frame_map: %d aggregatedupto: " INT64_FORMAT " aggregatedbase: " INT64_FORMAT,
+				 get_reduced_frame_map(winstate,
+									   winstate->aggregatedupto),
+				 winstate->aggregatedupto,
+				 winstate->aggregatedbase);
+#endif
+			/*
+			 * If the row status at currentpos is already decided and current
+			 * row status is not decided yet, it means we passed the last
+			 * reduced frame. Time to break the loop.
+			 */
+			if (get_reduced_frame_map(winstate,
+									  winstate->currentpos) != RF_NOT_DETERMINED &&
+				get_reduced_frame_map(winstate,
+									  winstate->aggregatedupto) == RF_NOT_DETERMINED)
+				break;
+
+			/*
+			 * Otherwise we need to calculate the reduced frame.
+			 */
+			ret = row_is_in_reduced_frame(winstate->agg_winobj,
+										  winstate->aggregatedupto);
+			if (ret == -1)		/* unmatched row */
+				break;
+
+			/*
+			 * Check if current row needs to be skipped due to no match.
+			 */
+			if (get_reduced_frame_map(winstate,
+									  winstate->aggregatedupto) == RF_SKIPPED &&
+				winstate->aggregatedupto == winstate->aggregatedbase)
+			{
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "skip current row for aggregation");
+#endif
+				break;
+			}
+		}
+
 		/* Set tuple context for evaluation of aggregate arguments */
 		winstate->tmpcontext->ecxt_outertuple = agg_row_slot;
 
@@ -976,6 +1130,7 @@ next_tuple:
 		ExecClearTuple(agg_row_slot);
 	}
 
+
 	/* The frame's end is not supposed to move backwards, ever */
 	Assert(aggregatedupto_nonrestarted <= winstate->aggregatedupto);
 
@@ -995,7 +1150,6 @@ next_tuple:
 								 &winstate->perfunc[wfuncno],
 								 peraggstate,
 								 result, isnull);
-
 		/*
 		 * save the result in case next row shares the same frame.
 		 *
@@ -1090,6 +1244,7 @@ begin_partition(WindowAggState *winstate)
 	winstate->framehead_valid = false;
 	winstate->frametail_valid = false;
 	winstate->grouptail_valid = false;
+	create_reduced_frame_map(winstate);
 	winstate->spooled_rows = 0;
 	winstate->currentpos = 0;
 	winstate->frameheadpos = 0;
@@ -2053,6 +2208,11 @@ ExecWindowAgg(PlanState *pstate)
 
 	CHECK_FOR_INTERRUPTS();
 
+#ifdef RPR_DEBUG
+	elog(DEBUG1, "ExecWindowAgg called. pos: " INT64_FORMAT,
+		 winstate->currentpos);
+#endif
+
 	if (winstate->status == WINDOWAGG_DONE)
 		return NULL;
 
@@ -2221,6 +2381,17 @@ ExecWindowAgg(PlanState *pstate)
 		/* don't evaluate the window functions when we're in pass-through mode */
 		if (winstate->status == WINDOWAGG_RUN)
 		{
+			/*
+			 * If RPR is defined and skip mode is next row, we need to clear
+			 * existing reduced frame info so that we newly calculate the info
+			 * starting from current row.
+			 */
+			if (rpr_is_defined(winstate))
+			{
+				if (winstate->rpSkipTo == ST_NEXT_ROW)
+					clear_reduced_frame_map(winstate);
+			}
+
 			/*
 			 * Evaluate true window functions
 			 */
@@ -2388,6 +2559,9 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	TupleDesc	scanDesc;
 	ListCell   *l;
 
+	TargetEntry *te;
+	Expr	   *expr;
+
 	/* check for unsupported flags */
 	Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
 
@@ -2486,6 +2660,16 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	winstate->temp_slot_2 = ExecInitExtraTupleSlot(estate, scanDesc,
 												   &TTSOpsMinimalTuple);
 
+	winstate->prev_slot = ExecInitExtraTupleSlot(estate, scanDesc,
+												 &TTSOpsMinimalTuple);
+
+	winstate->next_slot = ExecInitExtraTupleSlot(estate, scanDesc,
+												 &TTSOpsMinimalTuple);
+
+	winstate->null_slot = ExecInitExtraTupleSlot(estate, scanDesc,
+												 &TTSOpsMinimalTuple);
+	winstate->null_slot = ExecStoreAllNullTuple(winstate->null_slot);
+
 	/*
 	 * create frame head and tail slots only if needed (must create slots in
 	 * exactly the same cases that update_frameheadpos and update_frametailpos
@@ -2667,6 +2851,43 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	winstate->inRangeAsc = node->inRangeAsc;
 	winstate->inRangeNullsFirst = node->inRangeNullsFirst;
 
+	/* Set up SKIP TO type */
+	winstate->rpSkipTo = node->rpSkipTo;
+	/* Set up row pattern recognition PATTERN clause */
+	winstate->patternVariableList = node->patternVariable;
+	winstate->patternRegexpList = node->patternRegexp;
+
+	/* Set up row pattern recognition DEFINE clause */
+	winstate->defineInitial = node->defineInitial;
+	winstate->defineVariableList = NIL;
+	winstate->defineClauseList = NIL;
+	if (node->defineClause != NIL)
+	{
+		/*
+		 * Tweak arg var of PREV/NEXT so that it refers to scan/inner slot.
+		 */
+		foreach(l, node->defineClause)
+		{
+			char	   *name;
+			ExprState  *exps;
+
+			te = lfirst(l);
+			name = te->resname;
+			expr = te->expr;
+
+#ifdef RPR_DEBUG
+			elog(DEBUG1, "defineVariable name: %s", name);
+#endif
+			winstate->defineVariableList =
+				lappend(winstate->defineVariableList,
+						makeString(pstrdup(name)));
+			attno_map((Node *) expr);
+			exps = ExecInitExpr(expr, (PlanState *) winstate);
+			winstate->defineClauseList =
+				lappend(winstate->defineClauseList, exps);
+		}
+	}
+
 	winstate->all_first = true;
 	winstate->partition_spooled = false;
 	winstate->more_partitions = false;
@@ -2674,6 +2895,64 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	return winstate;
 }
 
+/*
+ * Rewrite varno of Var node that is the argument of PREV/NET so that it sees
+ * scan tuple (PREV) or inner tuple (NEXT).
+ */
+static void
+attno_map(Node *node)
+{
+	(void) expression_tree_walker(node, attno_map_walker, NULL);
+}
+
+static bool
+attno_map_walker(Node *node, void *context)
+{
+	FuncExpr   *func;
+	int			nargs;
+	Expr	   *expr;
+	Var		   *var;
+
+	if (node == NULL)
+		return false;
+
+	if (IsA(node, FuncExpr))
+	{
+		func = (FuncExpr *) node;
+
+		if (func->funcid == F_PREV || func->funcid == F_NEXT)
+		{
+			/* sanity check */
+			nargs = list_length(func->args);
+			if (list_length(func->args) != 1)
+				elog(ERROR, "PREV/NEXT must have 1 argument but function %d has %d args",
+					 func->funcid, nargs);
+
+			expr = (Expr *) lfirst(list_head(func->args));
+			if (!IsA(expr, Var))
+				elog(ERROR, "PREV/NEXT's arg is not Var");	/* XXX: is it possible
+															 * that arg type is
+															 * Const? */
+			var = (Var *) expr;
+
+			if (func->funcid == F_PREV)
+
+				/*
+				 * Rewrite varno from OUTER_VAR to regular var no so that the
+				 * var references scan tuple.
+				 */
+				var->varno = var->varnosyn;
+			else
+				var->varno = INNER_VAR;
+
+#ifdef RPR_DEBUG
+			elog(DEBUG1, "PREV/NEXT's varno is rewritten to: %d", var->varno);
+#endif
+		}
+	}
+	return expression_tree_walker(node, attno_map_walker, NULL);
+}
+
 /* -----------------
  * ExecEndWindowAgg
  * -----------------
@@ -2723,6 +3002,8 @@ ExecReScanWindowAgg(WindowAggState *node)
 	ExecClearTuple(node->agg_row_slot);
 	ExecClearTuple(node->temp_slot_1);
 	ExecClearTuple(node->temp_slot_2);
+	ExecClearTuple(node->prev_slot);
+	ExecClearTuple(node->next_slot);
 	if (node->framehead_slot)
 		ExecClearTuple(node->framehead_slot);
 	if (node->frametail_slot)
@@ -3083,7 +3364,8 @@ window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot)
 		return false;
 
 	if (pos < winobj->markpos)
-		elog(ERROR, "cannot fetch row before WindowObject's mark position");
+		elog(ERROR, "cannot fetch row: " INT64_FORMAT " before WindowObject's mark position: " INT64_FORMAT,
+			 pos, winobj->markpos);
 
 	oldcontext = MemoryContextSwitchTo(winstate->ss.ps.ps_ExprContext->ecxt_per_query_memory);
 
@@ -3403,14 +3685,54 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
 	WindowAggState *winstate;
 	ExprContext *econtext;
 	TupleTableSlot *slot;
-	int64		abs_pos;
-	int64		mark_pos;
 
 	Assert(WindowObjectIsValid(winobj));
 	winstate = winobj->winstate;
 	econtext = winstate->ss.ps.ps_ExprContext;
 	slot = winstate->temp_slot_1;
 
+	if (WinGetSlotInFrame(winobj, slot,
+						  relpos, seektype, set_mark,
+						  isnull, isout) == 0)
+	{
+		econtext->ecxt_outertuple = slot;
+		return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
+							econtext, isnull);
+	}
+
+	if (isout)
+		*isout = true;
+	*isnull = true;
+	return (Datum) 0;
+}
+
+/*
+ * WinGetSlotInFrame
+ * slot: TupleTableSlot to store the result
+ * relpos: signed rowcount offset from the seek position
+ * seektype: WINDOW_SEEK_HEAD or WINDOW_SEEK_TAIL
+ * set_mark: If the row is found/in frame and set_mark is true, the mark is
+ *		moved to the row as a side-effect.
+ * isnull: output argument, receives isnull status of result
+ * isout: output argument, set to indicate whether target row position
+ *		is out of frame (can pass NULL if caller doesn't care about this)
+ *
+ * Returns 0 if we successfullt got the slot. false if out of frame.
+ * (also isout is set)
+ */
+static int
+WinGetSlotInFrame(WindowObject winobj, TupleTableSlot *slot,
+				  int relpos, int seektype, bool set_mark,
+				  bool *isnull, bool *isout)
+{
+	WindowAggState *winstate;
+	int64		abs_pos;
+	int64		mark_pos;
+	int			num_reduced_frame;
+
+	Assert(WindowObjectIsValid(winobj));
+	winstate = winobj->winstate;
+
 	switch (seektype)
 	{
 		case WINDOW_SEEK_CURRENT:
@@ -3477,11 +3799,25 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
 						 winstate->frameOptions);
 					break;
 			}
+			num_reduced_frame = row_is_in_reduced_frame(winobj,
+														winstate->frameheadpos);
+			if (num_reduced_frame < 0)
+				goto out_of_frame;
+			else if (num_reduced_frame > 0)
+				if (relpos >= num_reduced_frame)
+					goto out_of_frame;
 			break;
 		case WINDOW_SEEK_TAIL:
 			/* rejecting relpos > 0 is easy and simplifies code below */
 			if (relpos > 0)
 				goto out_of_frame;
+
+			/*
+			 * RPR cares about frame head pos. Need to call
+			 * update_frameheadpos
+			 */
+			update_frameheadpos(winstate);
+
 			update_frametailpos(winstate);
 			abs_pos = winstate->frametailpos - 1 + relpos;
 
@@ -3548,6 +3884,14 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
 					mark_pos = 0;	/* keep compiler quiet */
 					break;
 			}
+
+			num_reduced_frame = row_is_in_reduced_frame(winobj,
+														winstate->frameheadpos + relpos);
+			if (num_reduced_frame < 0)
+				goto out_of_frame;
+			else if (num_reduced_frame > 0)
+				abs_pos = winstate->frameheadpos + relpos +
+					num_reduced_frame - 1;
 			break;
 		default:
 			elog(ERROR, "unrecognized window seek type: %d", seektype);
@@ -3566,15 +3910,13 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
 		*isout = false;
 	if (set_mark)
 		WinSetMarkPosition(winobj, mark_pos);
-	econtext->ecxt_outertuple = slot;
-	return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
-						econtext, isnull);
+	return 0;
 
 out_of_frame:
 	if (isout)
 		*isout = true;
 	*isnull = true;
-	return (Datum) 0;
+	return -1;
 }
 
 /*
@@ -3605,3 +3947,1251 @@ WinGetFuncArgCurrent(WindowObject winobj, int argno, bool *isnull)
 	return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
 						econtext, isnull);
 }
+
+/*
+ * rpr_is_defined
+ * return true if Row pattern recognition is defined.
+ */
+static
+bool
+rpr_is_defined(WindowAggState *winstate)
+{
+	return winstate->patternVariableList != NIL;
+}
+
+/*
+ * -----------------
+ * row_is_in_reduced_frame
+ * Determine whether a row is in the current row's reduced window frame
+ * according to row pattern matching
+ *
+ * The row must has been already determined that it is in a full window frame
+ * and fetched it into slot.
+ *
+ * Returns:
+ * = 0, RPR is not defined.
+ * >0, if the row is the first in the reduced frame. Return the number of rows
+ * in the reduced frame.
+ * -1, if the row is unmatched row
+ * -2, if the row is in the reduced frame but needed to be skipped because of
+ * AFTER MATCH SKIP PAST LAST ROW
+ * -----------------
+ */
+static
+int
+row_is_in_reduced_frame(WindowObject winobj, int64 pos)
+{
+	WindowAggState *winstate = winobj->winstate;
+	int			state;
+	int			rtn;
+
+	if (!rpr_is_defined(winstate))
+	{
+		/*
+		 * RPR is not defined. Assume that we are always in the the reduced
+		 * window frame.
+		 */
+		rtn = 0;
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "row_is_in_reduced_frame returns %d: pos: " INT64_FORMAT,
+			 rtn, pos);
+#endif
+		return rtn;
+	}
+
+	state = get_reduced_frame_map(winstate, pos);
+
+	if (state == RF_NOT_DETERMINED)
+	{
+		update_frameheadpos(winstate);
+		update_reduced_frame(winobj, pos);
+	}
+
+	state = get_reduced_frame_map(winstate, pos);
+
+	switch (state)
+	{
+			int64		i;
+			int			num_reduced_rows;
+
+		case RF_FRAME_HEAD:
+			num_reduced_rows = 1;
+			for (i = pos + 1;
+				 get_reduced_frame_map(winstate, i) == RF_SKIPPED; i++)
+				num_reduced_rows++;
+			rtn = num_reduced_rows;
+			break;
+
+		case RF_SKIPPED:
+			rtn = -2;
+			break;
+
+		case RF_UNMATCHED:
+			rtn = -1;
+			break;
+
+		default:
+			elog(ERROR, "Unrecognized state: %d at: " INT64_FORMAT,
+				 state, pos);
+			break;
+	}
+
+#ifdef RPR_DEBUG
+	elog(DEBUG1, "row_is_in_reduced_frame returns %d: pos: " INT64_FORMAT,
+		 rtn, pos);
+#endif
+	return rtn;
+}
+
+#define REDUCED_FRAME_MAP_INIT_SIZE	1024L
+
+/*
+ * create_reduced_frame_map
+ * Create reduced frame map
+ */
+static
+void
+create_reduced_frame_map(WindowAggState *winstate)
+{
+	winstate->reduced_frame_map =
+		MemoryContextAlloc(winstate->partcontext,
+						   REDUCED_FRAME_MAP_INIT_SIZE);
+	winstate->alloc_sz = REDUCED_FRAME_MAP_INIT_SIZE;
+	clear_reduced_frame_map(winstate);
+}
+
+/*
+ * clear_reduced_frame_map
+ * Clear reduced frame map
+ */
+static
+void
+clear_reduced_frame_map(WindowAggState *winstate)
+{
+	Assert(winstate->reduced_frame_map != NULL);
+	MemSet(winstate->reduced_frame_map, RF_NOT_DETERMINED,
+		   winstate->alloc_sz);
+}
+
+/*
+ * get_reduced_frame_map
+ * Get reduced frame map specified by pos
+ */
+static
+int
+get_reduced_frame_map(WindowAggState *winstate, int64 pos)
+{
+	Assert(winstate->reduced_frame_map != NULL);
+
+	if (pos < 0 || pos >= winstate->alloc_sz)
+		elog(ERROR, "wrong pos: " INT64_FORMAT, pos);
+
+	return winstate->reduced_frame_map[pos];
+}
+
+/*
+ * register_reduced_frame_map
+ * Add/replace reduced frame map member at pos.
+ * If there's no enough space, expand the map.
+ */
+static
+void
+register_reduced_frame_map(WindowAggState *winstate, int64 pos, int val)
+{
+	int64		realloc_sz;
+
+	Assert(winstate->reduced_frame_map != NULL);
+
+	if (pos < 0)
+		elog(ERROR, "wrong pos: " INT64_FORMAT, pos);
+
+	if (pos > winstate->alloc_sz - 1)
+	{
+		realloc_sz = winstate->alloc_sz * 2;
+
+		winstate->reduced_frame_map =
+			repalloc(winstate->reduced_frame_map, realloc_sz);
+
+		MemSet(winstate->reduced_frame_map + winstate->alloc_sz,
+			   RF_NOT_DETERMINED, realloc_sz - winstate->alloc_sz);
+
+		winstate->alloc_sz = realloc_sz;
+	}
+
+	winstate->reduced_frame_map[pos] = val;
+}
+
+/*
+ * update_reduced_frame
+ *		Update reduced frame info.
+ */
+static
+void
+update_reduced_frame(WindowObject winobj, int64 pos)
+{
+	WindowAggState *winstate = winobj->winstate;
+	ListCell   *lc1,
+			   *lc2;
+	bool		expression_result;
+	int			num_matched_rows;
+	int64		original_pos;
+	bool		anymatch;
+	StringInfo	encoded_str;
+	StringInfo	pattern_str = makeStringInfo();
+	StringSet  *str_set;
+	int			initial_index;
+	VariablePos *variable_pos;
+	bool		greedy = false;
+	int64		result_pos,
+				i;
+
+	/*
+	 * Set of pattern variables evaluated to true. Each character corresponds
+	 * to pattern variable. Example: str_set[0] = "AB"; str_set[1] = "AC"; In
+	 * this case at row 0 A and B are true, and A and C are true in row 1.
+	 */
+
+	/* initialize pattern variables set */
+	str_set = string_set_init();
+
+	/* save original pos */
+	original_pos = pos;
+
+	/*
+	 * Check if the pattern does not include any greedy quantifier. If it does
+	 * not, we can just apply the pattern to each row. If it succeeds, we are
+	 * done.
+	 */
+	foreach(lc1, winstate->patternRegexpList)
+	{
+		char	   *quantifier = strVal(lfirst(lc1));
+
+		if (*quantifier == '+' || *quantifier == '*')
+		{
+			greedy = true;
+			break;
+		}
+	}
+
+	/*
+	 * Non greedy case
+	 */
+	if (!greedy)
+	{
+		num_matched_rows = 0;
+
+		foreach(lc1, winstate->patternVariableList)
+		{
+			char	   *vname = strVal(lfirst(lc1));
+
+			encoded_str = makeStringInfo();
+
+#ifdef RPR_DEBUG
+			elog(DEBUG1, "pos: " INT64_FORMAT " pattern vname: %s",
+				 pos, vname);
+#endif
+			expression_result = false;
+
+			/* evaluate row pattern against current row */
+			result_pos = evaluate_pattern(winobj, pos, vname,
+										  encoded_str, &expression_result);
+			if (!expression_result || result_pos < 0)
+			{
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "expression result is false or out of frame");
+#endif
+				register_reduced_frame_map(winstate, original_pos,
+										   RF_UNMATCHED);
+				return;
+			}
+			/* move to next row */
+			pos++;
+			num_matched_rows++;
+		}
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "pattern matched");
+#endif
+		register_reduced_frame_map(winstate, original_pos, RF_FRAME_HEAD);
+
+		for (i = original_pos + 1; i < original_pos + num_matched_rows; i++)
+		{
+			register_reduced_frame_map(winstate, i, RF_SKIPPED);
+		}
+		return;
+	}
+
+	/*
+	 * Greedy quantifiers included. Loop over until none of pattern matches or
+	 * encounters end of frame.
+	 */
+	for (;;)
+	{
+		result_pos = -1;
+
+		/*
+		 * Loop over each PATTERN variable.
+		 */
+		anymatch = false;
+		encoded_str = makeStringInfo();
+
+		forboth(lc1, winstate->patternVariableList, lc2,
+				winstate->patternRegexpList)
+		{
+			char	   *vname = strVal(lfirst(lc1));
+#ifdef RPR_DEBUG
+			char	   *quantifier = strVal(lfirst(lc2));
+
+			elog(DEBUG1, "pos: " INT64_FORMAT " pattern vname: %s quantifier: %s",
+				 pos, vname, quantifier);
+#endif
+			expression_result = false;
+
+			/* evaluate row pattern against current row */
+			result_pos = evaluate_pattern(winobj, pos, vname,
+										  encoded_str, &expression_result);
+			if (expression_result)
+			{
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "expression result is true");
+#endif
+				anymatch = true;
+			}
+
+			/*
+			 * If out of frame, we are done.
+			 */
+			if (result_pos < 0)
+				break;
+		}
+
+		if (!anymatch)
+		{
+			/* none of patterns matched. */
+			break;
+		}
+
+		string_set_add(str_set, encoded_str);
+
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "pos: " INT64_FORMAT " encoded_str: %s",
+			 encoded_str->data);
+#endif
+
+		/* move to next row */
+		pos++;
+
+		if (result_pos < 0)
+		{
+			/* out of frame */
+			break;
+		}
+	}
+
+	if (string_set_get_size(str_set) == 0)
+	{
+		/* no match found in the first row */
+		register_reduced_frame_map(winstate, original_pos, RF_UNMATCHED);
+		return;
+	}
+
+#ifdef RPR_DEBUG
+	elog(DEBUG2, "pos: " INT64_FORMAT " encoded_str: %s",
+		 pos, encoded_str->data);
+#endif
+
+	/* build regular expression */
+	pattern_str = makeStringInfo();
+	appendStringInfoChar(pattern_str, '^');
+	initial_index = 0;
+
+	variable_pos = variable_pos_init();
+
+	forboth(lc1, winstate->patternVariableList,
+			lc2, winstate->patternRegexpList)
+	{
+		char	   *vname = strVal(lfirst(lc1));
+		char	   *quantifier = strVal(lfirst(lc2));
+		char		initial;
+
+		initial = pattern_initial(winstate, vname);
+		Assert(initial != 0);
+		appendStringInfoChar(pattern_str, initial);
+		if (quantifier[0])
+			appendStringInfoChar(pattern_str, quantifier[0]);
+
+		/*
+		 * Register the initial at initial_index. If the initial appears more
+		 * than once, all of it's initial_index will be recorded. This could
+		 * happen if a pattern variable appears in the PATTERN clause more
+		 * than once like "UP DOWN UP" "UP UP UP".
+		 */
+		variable_pos_register(variable_pos, initial, initial_index);
+
+		initial_index++;
+	}
+
+#ifdef RPR_DEBUG
+	elog(DEBUG2, "pos: " INT64_FORMAT " pattern: %s",
+		 pos, pattern_str->data);
+#endif
+
+	/* look for matching pattern variable sequence */
+#ifdef RPR_DEBUG
+	elog(DEBUG1, "search_str_set started");
+#endif
+	num_matched_rows = search_str_set(pattern_str->data,
+									  str_set, variable_pos);
+#ifdef RPR_DEBUG
+	elog(DEBUG1, "search_str_set returns: %d", num_matched_rows);
+#endif
+	variable_pos_discard(variable_pos);
+	string_set_discard(str_set);
+
+	/*
+	 * We are at the first row in the reduced frame.  Save the number of
+	 * matched rows as the number of rows in the reduced frame.
+	 */
+	if (num_matched_rows <= 0)
+	{
+		/* no match */
+		register_reduced_frame_map(winstate, original_pos, RF_UNMATCHED);
+	}
+	else
+	{
+		register_reduced_frame_map(winstate, original_pos, RF_FRAME_HEAD);
+
+		for (i = original_pos + 1; i < original_pos + num_matched_rows; i++)
+		{
+			register_reduced_frame_map(winstate, i, RF_SKIPPED);
+		}
+	}
+
+	return;
+}
+
+/*
+ * search_str_set
+ * Perform pattern matching using "pattern" against str_set. pattern is a
+ * regular expression derived from PATTERN clause. Note that the regular
+ * expression string is prefixed by '^' and followed by initials represented
+ * in a same way as str_set. str_set is a set of StringInfo. Each StringInfo
+ * has a string comprising initials of pattern variable strings being true in
+ * a row. The initials are one of [a-y], parallel to the order of variable
+ * names in DEFINE clause. Suppose DEFINE has variables START, UP and DOWN. If
+ * PATTERN has START, UP+ and DOWN, then the initials in PATTERN will be 'a',
+ * 'b' and 'c'. The "pattern" will be "^ab+c".
+ *
+ * variable_pos is an array representing the order of pattern variable string
+ * initials in PATTERN clause.  For example initial 'a' potion is in
+ * variable_pos[0].pos[0] = 0. Note that if the pattern is "START UP DOWN UP"
+ * (UP appears twice), then "UP" (initial is 'b') has two position 1 and
+ * 3. Thus variable_pos for b is variable_pos[1].pos[0] = 1 and
+ * variable_pos[1].pos[1] = 3.
+ *
+ * Returns the longest number of the matching rows (greedy matching) if
+ * quatifier '+' or '*' is included in "pattern".
+ */
+static
+int
+search_str_set(char *pattern, StringSet * str_set, VariablePos * variable_pos)
+{
+#define	MAX_CANDIDATE_NUM	10000	/* max pattern match candidate size */
+#define	FREEZED_CHAR	'Z'		/* a pattern is freezed if it ends with the
+								 * char */
+#define	DISCARD_CHAR	'z'		/* a pattern is not need to keep */
+
+	int			set_size;		/* number of rows in the set */
+	int			resultlen;
+	int			index;
+	StringSet  *old_str_set,
+			   *new_str_set;
+	int			new_str_size;
+	int			len;
+
+	set_size = string_set_get_size(str_set);
+	new_str_set = string_set_init();
+	len = 0;
+	resultlen = 0;
+
+	/*
+	 * Generate all possible pattern variable name initials as a set of
+	 * StringInfo named "new_str_set".  For example, if we have two rows
+	 * having "ab" (row 0) and "ac" (row 1) in the input str_set, new_str_set
+	 * will have set of StringInfo "aa", "ac", "ba" and "bc" in the end.
+	 */
+#ifdef RPR_DEBUG
+	elog(DEBUG1, "pattern: %s set_size: %d", pattern, set_size);
+#endif
+	for (index = 0; index < set_size; index++)
+	{
+		StringInfo	str;		/* search target row */
+		char	   *p;
+		int			old_set_size;
+		int			i;
+
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "index: %d", index);
+#endif
+		if (index == 0)
+		{
+			/* copy variables in row 0 */
+			str = string_set_get(str_set, index);
+			p = str->data;
+
+			/*
+			 * Loop over each new pattern variable char.
+			 */
+			while (*p)
+			{
+				StringInfo	new = makeStringInfo();
+
+				/* add pattern variable char */
+				appendStringInfoChar(new, *p);
+				/* add new one to string set */
+				string_set_add(new_str_set, new);
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "old_str: NULL new_str: %s", new->data);
+#endif
+				p++;			/* next pattern variable */
+			}
+		}
+		else					/* index != 0 */
+		{
+			old_str_set = new_str_set;
+			new_str_set = string_set_init();
+			str = string_set_get(str_set, index);
+			old_set_size = string_set_get_size(old_str_set);
+
+			/*
+			 * Loop over each rows in the previous result set.
+			 */
+			for (i = 0; i < old_set_size; i++)
+			{
+				StringInfo	new;
+				char		last_old_char;
+				int			old_str_len;
+				StringInfo	old = string_set_get(old_str_set, i);
+
+				p = old->data;
+				old_str_len = strlen(p);
+				if (old_str_len > 0)
+					last_old_char = p[old_str_len - 1];
+				else
+					last_old_char = '\0';
+
+				/* Is this old set freezed? */
+				if (last_old_char == FREEZED_CHAR)
+				{
+					/* if shorter match. we can discard it */
+					if ((old_str_len - 1) < resultlen)
+					{
+#ifdef RPR_DEBUG
+						elog(DEBUG1, "discard this old set because shorter match: %s",
+							 old->data);
+#endif
+						continue;
+					}
+
+#ifdef RPR_DEBUG
+					elog(DEBUG1, "keep this old set: %s", old->data);
+#endif
+
+					/* move the old set to new_str_set */
+					string_set_add(new_str_set, old);
+					old_str_set->str_set[i] = NULL;
+					continue;
+				}
+				/* Can this old set be discarded? */
+				else if (last_old_char == DISCARD_CHAR)
+				{
+#ifdef RPR_DEBUG
+					elog(DEBUG1, "discard this old set: %s", old->data);
+#endif
+					continue;
+				}
+
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "str->data: %s", str->data);
+#endif
+
+				/*
+				 * loop over each pattern variable initial char in the input
+				 * set.
+				 */
+				for (p = str->data; *p; p++)
+				{
+					/*
+					 * Optimization.  Check if the row's pattern variable
+					 * initial character position is greater than or equal to
+					 * the old set's last pattern variable initial character
+					 * position. For example, if the old set's last pattern
+					 * variable initials are "ab", then the new pattern
+					 * variable initial can be "b" or "c" but can not be "a",
+					 * if the initials in PATTERN is something like "a b c" or
+					 * "a b+ c+" etc.  This optimization is possible when we
+					 * only allow "+" quantifier.
+					 */
+					if (variable_pos_compare(variable_pos, last_old_char, *p))
+					{
+						/* copy source string */
+						new = makeStringInfo();
+						enlargeStringInfo(new, old->len + 1);
+						appendStringInfoString(new, old->data);
+						/* add pattern variable char */
+						appendStringInfoChar(new, *p);
+#ifdef RPR_DEBUG
+						elog(DEBUG1, "old_str: %s new_str: %s",
+							 old->data, new->data);
+#endif
+
+						/*
+						 * Adhoc optimization. If the first letter in the
+						 * input string is the first and second position one
+						 * and there's no associated quatifier '+', then we
+						 * can dicard the input because there's no chace to
+						 * expand the string further.
+						 *
+						 * For example, pattern "abc" cannot match "aa".
+						 */
+#ifdef RPR_DEBUG
+						elog(DEBUG1, "pattern[1]:%c pattern[2]:%c new[0]:%c new[1]:%c",
+							 pattern[1], pattern[2], new->data[0], new->data[1]);
+#endif
+						if (pattern[1] == new->data[0] &&
+							pattern[1] == new->data[1] &&
+							pattern[2] != '+' &&
+							pattern[1] != pattern[2])
+						{
+#ifdef RPR_DEBUG
+							elog(DEBUG1, "discard this new data: %s",
+								 new->data);
+#endif
+							pfree(new->data);
+							pfree(new);
+							continue;
+						}
+
+						/* add new one to string set */
+						string_set_add(new_str_set, new);
+					}
+					else
+					{
+						/*
+						 * We are freezing this pattern string.  Since there's
+						 * no chance to expand the string further, we perform
+						 * pattern matching against the string. If it does not
+						 * match, we can discard it.
+						 */
+						len = do_pattern_match(pattern, old->data);
+
+						if (len <= 0)
+						{
+							/* no match. we can discard it */
+							continue;
+						}
+
+						else if (len <= resultlen)
+						{
+							/* shorter match. we can discard it */
+							continue;
+						}
+						else
+						{
+							/* match length is the longest so far */
+
+							int			new_index;
+
+							/* remember the longest match */
+							resultlen = len;
+
+							/* freeze the pattern string */
+							new = makeStringInfo();
+							enlargeStringInfo(new, old->len + 1);
+							appendStringInfoString(new, old->data);
+							/* add freezed mark */
+							appendStringInfoChar(new, FREEZED_CHAR);
+#ifdef RPR_DEBUG
+							elog(DEBUG1, "old_str: %s new_str: %s", old->data, new->data);
+#endif
+							string_set_add(new_str_set, new);
+
+							/*
+							 * Search new_str_set to find out freezed entries
+							 * that have shorter match length. Mark them as
+							 * "discard" so that they are discarded in the
+							 * next round.
+							 */
+
+							/* new_index_size should be the one before */
+							new_str_size =
+								string_set_get_size(new_str_set) - 1;
+
+							/* loop over new_str_set */
+							for (new_index = 0; new_index < new_str_size;
+								 new_index++)
+							{
+								char		new_last_char;
+								int			new_str_len;
+
+								new = string_set_get(new_str_set, new_index);
+								new_str_len = strlen(new->data);
+								if (new_str_len > 0)
+								{
+									new_last_char =
+										new->data[new_str_len - 1];
+									if (new_last_char == FREEZED_CHAR &&
+										(new_str_len - 1) <= len)
+									{
+										/*
+										 * mark this set to discard in the
+										 * next round
+										 */
+										appendStringInfoChar(new, DISCARD_CHAR);
+#ifdef RPR_DEBUG
+										elog(DEBUG1, "add discard char: %s", new->data);
+#endif
+									}
+								}
+							}
+						}
+					}
+				}
+			}
+			/* we no longer need old string set */
+			string_set_discard(old_str_set);
+		}
+	}
+
+	/*
+	 * Perform pattern matching to find out the longest match.
+	 */
+	new_str_size = string_set_get_size(new_str_set);
+#ifdef RPR_DEBUG
+	elog(DEBUG1, "new_str_size: %d", new_str_size);
+#endif
+	len = 0;
+	resultlen = 0;
+
+	for (index = 0; index < new_str_size; index++)
+	{
+		StringInfo	s;
+
+		s = string_set_get(new_str_set, index);
+		if (s == NULL)
+			continue;			/* no data */
+
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "target string: %s", s->data);
+#endif
+		len = do_pattern_match(pattern, s->data);
+		if (len > resultlen)
+		{
+			/* remember the longest match */
+			resultlen = len;
+
+			/*
+			 * If the size of result set is equal to the number of rows in the
+			 * set, we are done because it's not possible that the number of
+			 * matching rows exceeds the number of rows in the set.
+			 */
+			if (resultlen >= set_size)
+				break;
+		}
+	}
+
+	/* we no longer need new string set */
+	string_set_discard(new_str_set);
+
+	return resultlen;
+}
+
+/*
+ * do_pattern_match
+ * perform pattern match using pattern against encoded_str.
+ * returns matching number of rows if matching is succeeded.
+ * Otherwise returns 0.
+ */
+static
+int
+do_pattern_match(char *pattern, char *encoded_str)
+{
+	Datum		d;
+	text	   *res;
+	char	   *substr;
+	int			len = 0;
+	text	   *pattern_text,
+			   *encoded_str_text;
+
+	pattern_text = cstring_to_text(pattern);
+	encoded_str_text = cstring_to_text(encoded_str);
+
+	/*
+	 * We first perform pattern matching using regexp_instr, then call
+	 * textregexsubstr to get matched substring to know how long the matched
+	 * string is. That is the number of rows in the reduced window frame.  The
+	 * reason why we can't call textregexsubstr in the first place is, it
+	 * errors out if pattern does not match.
+	 */
+	if (DatumGetInt32(DirectFunctionCall2Coll(
+						  regexp_instr, DEFAULT_COLLATION_OID,
+						  PointerGetDatum(encoded_str_text),
+						  PointerGetDatum(pattern_text))))
+	{
+		d = DirectFunctionCall2Coll(textregexsubstr,
+									DEFAULT_COLLATION_OID,
+									PointerGetDatum(encoded_str_text),
+									PointerGetDatum(pattern_text));
+		if (d != 0)
+		{
+			res = DatumGetTextPP(d);
+			substr = text_to_cstring(res);
+			len = strlen(substr);
+			pfree(substr);
+		}
+	}
+	pfree(encoded_str_text);
+	pfree(pattern_text);
+
+	return len;
+}
+
+/*
+ * evaluate_pattern
+ * Evaluate expression associated with PATTERN variable vname.  current_pos is
+ * relative row position in a frame (starting from 0). If vname is evaluated
+ * to true, initial letters associated with vname is appended to
+ * encode_str. result is out paramater representing the expression evaluation
+ * result is true of false.
+ *---------
+ * Return values are:
+ * >=0: the last match absolute row position
+ * otherwise out of frame.
+ *---------
+ */
+static
+int64
+evaluate_pattern(WindowObject winobj, int64 current_pos,
+				 char *vname, StringInfo encoded_str, bool *result)
+{
+	WindowAggState *winstate = winobj->winstate;
+	ExprContext *econtext = winstate->ss.ps.ps_ExprContext;
+	ListCell   *lc1,
+			   *lc2,
+			   *lc3;
+	ExprState  *pat;
+	Datum		eval_result;
+	bool		out_of_frame = false;
+	bool		isnull;
+	TupleTableSlot *slot;
+
+	forthree(lc1, winstate->defineVariableList,
+			 lc2, winstate->defineClauseList,
+			 lc3, winstate->defineInitial)
+	{
+		char		initial;	/* initial letter associated with vname */
+		char	   *name = strVal(lfirst(lc1));
+
+		if (strcmp(vname, name))
+			continue;
+
+		initial = *(strVal(lfirst(lc3)));
+
+		/* set expression to evaluate */
+		pat = lfirst(lc2);
+
+		/* get current, previous and next tuples */
+		if (!get_slots(winobj, current_pos))
+		{
+			out_of_frame = true;
+		}
+		else
+		{
+			/* evaluate the expression */
+			eval_result = ExecEvalExpr(pat, econtext, &isnull);
+			if (isnull)
+			{
+				/* expression is NULL */
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "expression for %s is NULL at row: " INT64_FORMAT,
+					 vname, current_pos);
+#endif
+				*result = false;
+			}
+			else
+			{
+				if (!DatumGetBool(eval_result))
+				{
+					/* expression is false */
+#ifdef RPR_DEBUG
+					elog(DEBUG1, "expression for %s is false at row: " INT64_FORMAT,
+						 vname, current_pos);
+#endif
+					*result = false;
+				}
+				else
+				{
+					/* expression is true */
+#ifdef RPR_DEBUG
+					elog(DEBUG1, "expression for %s is true at row: " INT64_FORMAT,
+						 vname, current_pos);
+#endif
+					appendStringInfoChar(encoded_str, initial);
+					*result = true;
+				}
+			}
+
+			slot = winstate->temp_slot_1;
+			if (slot != winstate->null_slot)
+				ExecClearTuple(slot);
+			slot = winstate->prev_slot;
+			if (slot != winstate->null_slot)
+				ExecClearTuple(slot);
+			slot = winstate->next_slot;
+			if (slot != winstate->null_slot)
+				ExecClearTuple(slot);
+
+			break;
+		}
+
+		if (out_of_frame)
+		{
+			*result = false;
+			return -1;
+		}
+	}
+	return current_pos;
+}
+
+/*
+ * get_slots
+ * Get current, previous and next tuples.
+ * Returns false if current row is out of partition/full frame.
+ */
+static
+bool
+get_slots(WindowObject winobj, int64 current_pos)
+{
+	WindowAggState *winstate = winobj->winstate;
+	TupleTableSlot *slot;
+	int			ret;
+	ExprContext *econtext;
+
+	econtext = winstate->ss.ps.ps_ExprContext;
+
+	/* set up current row tuple slot */
+	slot = winstate->temp_slot_1;
+	if (!window_gettupleslot(winobj, current_pos, slot))
+	{
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "current row is out of partition at:" INT64_FORMAT,
+			 current_pos);
+#endif
+		return false;
+	}
+	ret = row_is_in_frame(winstate, current_pos, slot);
+	if (ret <= 0)
+	{
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "current row is out of frame at: " INT64_FORMAT,
+			 current_pos);
+#endif
+		ExecClearTuple(slot);
+		return false;
+	}
+	econtext->ecxt_outertuple = slot;
+
+	/* for PREV */
+	if (current_pos > 0)
+	{
+		slot = winstate->prev_slot;
+		if (!window_gettupleslot(winobj, current_pos - 1, slot))
+		{
+#ifdef RPR_DEBUG
+			elog(DEBUG1, "previous row is out of partition at: " INT64_FORMAT,
+				 current_pos - 1);
+#endif
+			econtext->ecxt_scantuple = winstate->null_slot;
+		}
+		else
+		{
+			ret = row_is_in_frame(winstate, current_pos - 1, slot);
+			if (ret <= 0)
+			{
+#ifdef RPR_DEBUG
+				elog(DEBUG1, "previous row is out of frame at: " INT64_FORMAT,
+					 current_pos - 1);
+#endif
+				ExecClearTuple(slot);
+				econtext->ecxt_scantuple = winstate->null_slot;
+			}
+			else
+			{
+				econtext->ecxt_scantuple = slot;
+			}
+		}
+	}
+	else
+		econtext->ecxt_scantuple = winstate->null_slot;
+
+	/* for NEXT */
+	slot = winstate->next_slot;
+	if (!window_gettupleslot(winobj, current_pos + 1, slot))
+	{
+#ifdef RPR_DEBUG
+		elog(DEBUG1, "next row is out of partiton at: " INT64_FORMAT,
+			 current_pos + 1);
+#endif
+		econtext->ecxt_innertuple = winstate->null_slot;
+	}
+	else
+	{
+		ret = row_is_in_frame(winstate, current_pos + 1, slot);
+		if (ret <= 0)
+		{
+#ifdef RPR_DEBUG
+			elog(DEBUG1, "next row is out of frame at: " INT64_FORMAT,
+				 current_pos + 1);
+#endif
+			ExecClearTuple(slot);
+			econtext->ecxt_innertuple = winstate->null_slot;
+		}
+		else
+			econtext->ecxt_innertuple = slot;
+	}
+	return true;
+}
+
+/*
+ * pattern_initial
+ * Return pattern variable initial character
+ * matching with pattern variable name vname.
+ * If not found, return 0.
+ */
+static
+char
+pattern_initial(WindowAggState *winstate, char *vname)
+{
+	char		initial;
+	char	   *name;
+	ListCell   *lc1,
+			   *lc2;
+
+	forboth(lc1, winstate->defineVariableList,
+			lc2, winstate->defineInitial)
+	{
+		name = strVal(lfirst(lc1)); /* DEFINE variable name */
+		initial = *(strVal(lfirst(lc2)));	/* DEFINE variable initial */
+
+
+		if (!strcmp(name, vname))
+			return initial;		/* found */
+	}
+	return 0;
+}
+
+/*
+ * string_set_init
+ * Create dynamic set of StringInfo.
+ */
+static
+StringSet * string_set_init(void)
+{
+/* Initial allocation size of str_set */
+#define STRING_SET_ALLOC_SIZE	1024
+
+	StringSet  *string_set;
+	Size		set_size;
+
+	string_set = palloc0(sizeof(StringSet));
+	string_set->set_index = 0;
+	set_size = STRING_SET_ALLOC_SIZE;
+	string_set->str_set = palloc(set_size * sizeof(StringInfo));
+	string_set->set_size = set_size;
+
+	return string_set;
+}
+
+/*
+ * string_set_add
+ * Add StringInfo str to StringSet string_set.
+ */
+static
+void
+string_set_add(StringSet * string_set, StringInfo str)
+{
+	Size		set_size;
+
+	set_size = string_set->set_size;
+	if (string_set->set_index >= set_size)
+	{
+		set_size *= 2;
+		string_set->str_set = repalloc(string_set->str_set,
+									   set_size * sizeof(StringInfo));
+		string_set->set_size = set_size;
+	}
+
+	string_set->str_set[string_set->set_index++] = str;
+
+	return;
+}
+
+/*
+ * string_set_get
+ * Returns StringInfo specified by index.
+ * If there's no data yet, returns NULL.
+ */
+static
+StringInfo
+string_set_get(StringSet * string_set, int index)
+{
+	/* no data? */
+	if (index == 0 && string_set->set_index == 0)
+		return NULL;
+
+	if (index < 0 || index >= string_set->set_index)
+		elog(ERROR, "invalid index: %d", index);
+
+	return string_set->str_set[index];
+}
+
+/*
+ * string_set_get_size
+ * Returns the size of StringSet.
+ */
+static
+int
+string_set_get_size(StringSet * string_set)
+{
+	return string_set->set_index;
+}
+
+/*
+ * string_set_discard
+ * Discard StringSet.
+ * All memory including StringSet itself is freed.
+ */
+static
+void
+string_set_discard(StringSet * string_set)
+{
+	int			i;
+
+	for (i = 0; i < string_set->set_index; i++)
+	{
+		StringInfo	str = string_set->str_set[i];
+
+		if (str)
+		{
+			pfree(str->data);
+			pfree(str);
+		}
+	}
+	pfree(string_set->str_set);
+	pfree(string_set);
+}
+
+/*
+ * variable_pos_init
+ * Create and initialize variable postion structure
+ */
+static
+VariablePos * variable_pos_init(void)
+{
+	VariablePos *variable_pos;
+
+	variable_pos = palloc(sizeof(VariablePos) * NUM_ALPHABETS);
+	MemSet(variable_pos, -1, sizeof(VariablePos) * NUM_ALPHABETS);
+	return variable_pos;
+}
+
+/*
+ * variable_pos_register
+ * Register pattern variable whose initial is initial into postion index.
+ * pos is position of initial.
+ * If pos is already registered, register it at next empty slot.
+ */
+static
+void
+variable_pos_register(VariablePos * variable_pos, char initial, int pos)
+{
+	int			index = initial - 'a';
+	int			slot;
+	int			i;
+
+	if (pos < 0 || pos > NUM_ALPHABETS)
+		elog(ERROR, "initial is not valid char: %c", initial);
+
+	for (i = 0; i < NUM_ALPHABETS; i++)
+	{
+		slot = variable_pos[index].pos[i];
+		if (slot < 0)
+		{
+			/* empty slot found */
+			variable_pos[index].pos[i] = pos;
+			return;
+		}
+	}
+	elog(ERROR, "no empty slot for initial: %c", initial);
+}
+
+/*
+ * variable_pos_compare
+ * Returns true if initial1 can be followed by initial2
+ */
+static
+bool
+variable_pos_compare(VariablePos * variable_pos, char initial1, char initial2)
+{
+	int			index1,
+				index2;
+	int			pos1,
+				pos2;
+
+	for (index1 = 0;; index1++)
+	{
+		pos1 = variable_pos_fetch(variable_pos, initial1, index1);
+		if (pos1 < 0)
+			break;
+
+		for (index2 = 0;; index2++)
+		{
+			pos2 = variable_pos_fetch(variable_pos, initial2, index2);
+			if (pos2 < 0)
+				break;
+			if (pos1 <= pos2)
+				return true;
+		}
+	}
+	return false;
+}
+
+/*
+ * variable_pos_fetch
+ * Fetch position of pattern variable whose initial is initial, and whose index
+ * is index. If no postion was registered by initial, index, returns -1.
+ */
+static
+int
+variable_pos_fetch(VariablePos * variable_pos, char initial, int index)
+{
+	int			pos = initial - 'a';
+
+	if (pos < 0 || pos > NUM_ALPHABETS)
+		elog(ERROR, "initial is not valid char: %c", initial);
+
+	if (index < 0 || index > NUM_ALPHABETS)
+		elog(ERROR, "index is not valid: %d", index);
+
+	return variable_pos[pos].pos[index];
+}
+
+/*
+ * variable_pos_discard
+ * Discard VariablePos
+ */
+static
+void
+variable_pos_discard(VariablePos * variable_pos)
+{
+	pfree(variable_pos);
+}
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index 473c61569f..92c528d38c 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -13,6 +13,9 @@
  */
 #include "postgres.h"
 
+#include "catalog/pg_collation_d.h"
+#include "executor/executor.h"
+#include "nodes/execnodes.h"
 #include "nodes/parsenodes.h"
 #include "nodes/supportnodes.h"
 #include "utils/fmgrprotos.h"
@@ -37,11 +40,19 @@ typedef struct
 	int64		remainder;		/* (total rows) % (bucket num) */
 } ntile_context;
 
+/*
+ * rpr process information.
+ * Used for AFTER MATCH SKIP PAST LAST ROW
+ */
+typedef struct SkipContext
+{
+	int64		pos;			/* last row absolute position */
+}			SkipContext;
+
 static bool rank_up(WindowObject winobj);
 static Datum leadlag_common(FunctionCallInfo fcinfo,
 							bool forward, bool withoffset, bool withdefault);
 
-
 /*
  * utility routine for *_rank functions.
  */
@@ -674,7 +685,7 @@ window_last_value(PG_FUNCTION_ARGS)
 	bool		isnull;
 
 	result = WinGetFuncArgInFrame(winobj, 0,
-								  0, WINDOW_SEEK_TAIL, true,
+								  0, WINDOW_SEEK_TAIL, false,
 								  &isnull, NULL);
 	if (isnull)
 		PG_RETURN_NULL();
@@ -714,3 +725,25 @@ window_nth_value(PG_FUNCTION_ARGS)
 
 	PG_RETURN_DATUM(result);
 }
+
+/*
+ * prev
+ * Dummy function to invoke RPR's navigation operator "PREV".
+ * This is *not* a window function.
+ */
+Datum
+window_prev(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(PG_GETARG_DATUM(0));
+}
+
+/*
+ * next
+ * Dummy function to invoke RPR's navigation operation "NEXT".
+ * This is *not* a window function.
+ */
+Datum
+window_next(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(PG_GETARG_DATUM(0));
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6a5476d3c4..5e7506dabb 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10479,6 +10479,12 @@
 { oid => '3114', descr => 'fetch the Nth row value',
   proname => 'nth_value', prokind => 'w', prorettype => 'anyelement',
   proargtypes => 'anyelement int4', prosrc => 'window_nth_value' },
+{ oid => '6122', descr => 'previous value',
+  proname => 'prev', provolatile => 's', prorettype => 'anyelement',
+  proargtypes => 'anyelement', prosrc => 'window_prev' },
+{ oid => '6123', descr => 'next value',
+  proname => 'next', provolatile => 's', prorettype => 'anyelement',
+  proargtypes => 'anyelement', prosrc => 'window_next' },
 
 # functions for range types
 { oid => '3832', descr => 'I/O',
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 8bc421e7c0..4dd9a17eca 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -2554,6 +2554,11 @@ typedef enum WindowAggStatus
 									 * tuples during spool */
 } WindowAggStatus;
 
+#define	RF_NOT_DETERMINED	0
+#define	RF_FRAME_HEAD		1
+#define	RF_SKIPPED			2
+#define	RF_UNMATCHED		3
+
 typedef struct WindowAggState
 {
 	ScanState	ss;				/* its first field is NodeTag */
@@ -2602,6 +2607,19 @@ typedef struct WindowAggState
 	int64		groupheadpos;	/* current row's peer group head position */
 	int64		grouptailpos;	/* " " " " tail position (group end+1) */
 
+	/* these fields are used in Row pattern recognition: */
+	RPSkipTo	rpSkipTo;		/* Row Pattern Skip To type */
+	List	   *patternVariableList;	/* list of row pattern variables names
+										 * (list of String) */
+	List	   *patternRegexpList;	/* list of row pattern regular expressions
+									 * ('+' or ''. list of String) */
+	List	   *defineVariableList; /* list of row pattern definition
+									 * variables (list of String) */
+	List	   *defineClauseList;	/* expression for row pattern definition
+									 * search conditions ExprState list */
+	List	   *defineInitial;	/* list of row pattern definition variable
+								 * initials (list of String) */
+
 	MemoryContext partcontext;	/* context for partition-lifespan data */
 	MemoryContext aggcontext;	/* shared context for aggregate working data */
 	MemoryContext curaggcontext;	/* current aggregate's working data */
@@ -2638,6 +2656,18 @@ typedef struct WindowAggState
 	TupleTableSlot *agg_row_slot;
 	TupleTableSlot *temp_slot_1;
 	TupleTableSlot *temp_slot_2;
+
+	/* temporary slots for RPR */
+	TupleTableSlot *prev_slot;	/* PREV row navigation operator */
+	TupleTableSlot *next_slot;	/* NEXT row navigation operator */
+	TupleTableSlot *null_slot;	/* all NULL slot */
+
+	/*
+	 * Each byte corresponds to a row positioned at absolute its pos in
+	 * partition.  See above definition for RF_*
+	 */
+	char	   *reduced_frame_map;
+	int64		alloc_sz;		/* size of the map */
 } WindowAggState;
 
 /* ----------------
-- 
2.25.1


----Next_Part(Fri_May_24_11_39_19_2024_763)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v20-0006-Row-pattern-recognition-patch-docs.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread


end of thread, other threads:[~2026-06-30 19:38 UTC | newest]

Thread overview: 33+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-05-08 21:37 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Justin Pryzby <[email protected]>
2024-05-08 21:51 ` Alexander Korotkov <[email protected]>
2024-05-14 14:49   ` Justin Pryzby <[email protected]>
2024-05-17 10:05     ` Alexander Korotkov <[email protected]>
2024-05-17 11:02       ` Pavel Borisov <[email protected]>
2024-05-17 11:33         ` Alexander Korotkov <[email protected]>
2024-05-24 19:29           ` Tom Lane <[email protected]>
2024-05-24 20:00             ` Alexander Lakhin <[email protected]>
2024-05-25 12:53               ` Alexander Korotkov <[email protected]>
2024-05-26 03:58                 ` Alexander Korotkov <[email protected]>
2026-06-15 11:07       ` Justin Pryzby <[email protected]>
2026-06-15 20:50         ` Alexander Korotkov <[email protected]>
2026-06-15 21:36           ` Pavel Borisov <[email protected]>
2026-06-15 21:36             ` Alexander Korotkov <[email protected]>
2026-06-15 21:56               ` Pavel Borisov <[email protected]>
2026-06-16 12:22                 ` Alexander Korotkov <[email protected]>
2026-06-16 13:01                   ` Pavel Borisov <[email protected]>
2026-06-18 07:31                     ` Alexander Korotkov <[email protected]>
2026-06-18 15:49                       ` Justin Pryzby <[email protected]>
2026-06-21 12:11                         ` Alexander Korotkov <[email protected]>
2026-06-22 10:44                           ` Pavel Borisov <[email protected]>
2026-06-24 10:24                             ` Alexander Korotkov <[email protected]>
2026-06-24 10:44                               ` Pavel Borisov <[email protected]>
2026-06-26 13:40                       ` Justin Pryzby <[email protected]>
2026-06-26 13:47                         ` Pavel Borisov <[email protected]>
2026-06-26 15:20                           ` Dmitry Koval <[email protected]>
2026-06-26 19:50                             ` Alexander Korotkov <[email protected]>
2026-06-30 11:06                               ` Alexander Korotkov <[email protected]>
2026-06-30 11:25                                 ` Pavel Borisov <[email protected]>
2026-06-30 19:38                                   ` Alexander Korotkov <[email protected]>
2026-06-16 08:08               ` jian he <[email protected]>
2026-06-16 12:19                 ` Alexander Korotkov <[email protected]>
2024-05-24 02:26 [PATCH v20 5/8] Row pattern recognition patch (executor). Tatsuo Ishii <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox