GPDB7 Partitioning commands: remove guard for "one partition must remain"

[Alexandra Wang]

Hello hackers,

I've been playing with partitioning on GPDB7 with mixed flavors of

grammar: upstream Postgres grammar and legacy GPDB grammar. I have the

following findings and I'm looking for your thoughts.

## Context

Historically[1] Greenplum didn't allow a partitioned table to have

zero child. This guard exists in gpdb6 and all prior versions for

CREATE TABLE and ALTER TABLE DROP PARTITION. 

For instance, the following commands ERROR in GPDB6:

```sql

-- ERROR: no partitions specified at depth 1

create table foo (a int, b int) distributed by(a) partition by range(b);

-- This works

create table foo (a int, b int) distributed by(a) partition by range(b) (default partition def);

-- ERROR: cannot drop partition "def" of relation "foo" -- only one remains

-- HINT: Use DROP TABLE "foo" to remove the table and the final partition

alter table foo drop partition def;

```

In contrast, upstream Postgres does not have this restriction. A

partitioned table can be created with zero child, and the last child

partition can be dropped or detached from a parent table. 

For instance, the follow commands work in Postgres:

```sql

create table foo (a int, b int) distributed by(a) partition by range(b);

-- PG-style equivalent of ALTER TABLE ADD PARTITION

create table foo_def partition of foo default;

-- PG-style equivalent of ALTER TABLE DROP PARTITION

drop table foo_def;

-- PG-style command similar to ALTER TABLE DROP PARTITION, but

-- foo_def will continue to exist as a table outside of the partitioning hierarchy

alter table foo detach partition foo_def;

```

Currently in GPDB7, we keep Greenplum's historical behavior for legacy

GPDB syntax as much as we could, and adopt the Upstream behavior for

Postgres syntax. Because GPDB7 supports both flavors of partitioning

grammar, it is currently really inconsistent with whether or not to

allow a parent partitioned table to have zero children.

Below is a summary of relevant GPDB7 commands.

### CREATE TABLE - allows zero child

```sql

create table foo (a int, b int) distributed by(a) partition by range(b);

```

### ALTER TABLE ADD PARTITION - does NOT allow zero child

```sql

postgres=# alter table foo add partition foo_p1 start (1) end (3);

ERROR:  XX000: GPDB add partition syntax needs at least one sibling to exist (tablecmds_gp.c:1212)

LOCATION:  ATExecGPPartCmds, tablecmds_gp.c:1212

```

### ALTER TABLE ATTACH PARTITION - allows zero child

```sql

create table foo_p1 (like foo);

alter table foo attach partition foo_p1 for values from (1) to (3);

```

### ALTER TABLE DROP PARTITION - does NOT allow zero child

```sql

postgres=# alter table foo drop partition for (1);

ERROR: 2BP01: cannot drop partition "foo_p1" of "foo" -- only one remains

HINT: Use DROP TABLE "foo" to remove the table and the final partition

```

### ALTER TABLE DETACH PARTITION - allows zero child

```

alter table foo detach partition foo_p1;

```

### ALTER TABLE SET SUBPARTITION TEMPLATE - does NOT allow zero child

```

create table bar (a int, b int, c int) partition by list (b);

create table bar_p1 partition of bar for values in (1,2) partition by range(c);

-- ALTER TABLE SET SUBPARTITION TEMPLATE ERRORs if the grandparent/parent table has zero child.

-- This is an scenario we wouldn't encounter if user has only used legacy pre-GPDB6 syntax.

postgres=# alter table bar set subpartition template (subpartition sp1 start (1) end (3));

ERROR:  XX000: GPDB SET SUBPARTITION TEMPLATE syntax needs at least one sibling to exist (tablecmds_gp.c:1358)

LOCATION:  ATExecGPPartCmds, tablecmds_gp.c:1358

```

## Why do we care? (more context)

While I was working on

https://github.com/greenplum-db/gpdb/pull/14211, I noticed two things:

1. We don't have enough test coverage for mixed usage of Postgres and

legacy GPDB style partitioning syntax.

2. GPDB7's ALTER TABLE SET SUBPARTITION TEMPLATE and ALTER TABLE ADD

PARTITION commands are "luck-based" for a heterogeneous partitioning

hierarchy that can be only created using Postgres syntax.

Consider the following example:

```sql

-- Create top level parent

CREATE TABLE heteropart (id int, year date, letter char(1)) DISTRIBUTED BY (id, letter, year) PARTITION BY list (letter);

-- Add a level 1 partition that is NOT PARTITIONED. This for now is the FIRST level 1 partition.

CREATE TABLE heteropart_c PARTITION OF heteropart FOR VALUES IN ('C');

-- Add a level 1 partition that is partitioned BY RANGE with ZERO child. This now becomes the FIRST level 1 partition.

CREATE TABLE heteropart_b PARTITION OF heteropart FOR VALUES IN ('B') PARTITION BY range (year);

-- Add a level 2 child of heteropart_b.

CREATE TABLE heteropart_b_sp1 PARTITION OF heteropart_b FOR VALUES FROM (date '2021-01-01') TO (date '2022-01-01');

-- Add a level 1 partition that is partitioned BY LIST with ZERO child. This now becomes the FIRST level 1 partition.

CREATE TABLE heteropart_a PARTITION OF heteropart FOR VALUES IN ('A') PARTITION BY list (year);

-- Add a level 2 child of heteropart_a.

CREATE TABLE heteropart_a_sp1 PARTITION OF heteropart_a FOR VALUES IN (date '2022-01-01');

```

**You'd probably wonder how we decide who is the first sibling

partition: It depends on the partition bounds. In the above example,

the level 1 partitions are sorted by the list bound values, hence

`heteropart_a`(with VALUES IN ('A')) is the first partition, followed

by `heteropart_b` (with VALUES in 'B'), and then `heteropart_c`(with

VALUES in 'C'). (Refer to `qsort_partition_list_value_cmp()` and

`qsort_partition_rbound_cmp()` for more details.)

In the above example, the partitioning hierarchy is heterogeneous in

the sense that the sibling partitions on level 1 could be a leaf

table, a range partitioned table, or a list partitioned table, and the

portioned tables could also have zero children.

Now if we add a new partition using the legacy GPDB syntax:

```sql=

-- ERROR:  42P16: no partitions specified at depth 2

ALTER TABLE heteropart ADD PARTITION e VALUES ('E');

```

This ERRORed because according to the FIRST level 1 sibling partition

(`heteropart_a`), we needed a level 2 subpartition spec. Even though

we already have `heteropart_c` who does not have one.

SET SUBPARTITION TEMPLATE suffers similar problem:

```

-- ERROR:  42P16: invalid boundary specification for LIST partition

ALTER TABLE heteropart SET SUBPARTITION TEMPLATE(

    subpartition r1 START (date '2001-01-01') END (date '2003-01-01'),

    subpartition r2 START (date '2003-01-01') END (date '2005-01-01') EVERY (interval '1 year')

    );

```

This ERRORed because according to the FIRST level 1 sibling partition

(`heteropart_a`), the partition strategy is LIST. Even though we

already have `heteropart_b` who is partitioned by RANGE.

Now let's actually respect the FIRST level 1 partition

(`heteropart_a`) and set a subpartition template:

```

-- this works

ALTER TABLE heteropart SET SUBPARTITION TEMPLATE(

    subpartition r1 VALUES (date '2001-01-01')

    );

-- this also works

ALTER TABLE heteropart ADD PARTITION e VALUES ('E');

```

Now DROP the current FIRST level 1 partition `heteropart_a`.

```sql=

DROP TABLE heteropart_a;

```

Now that `heteropart_b` becomes the FIRST level 1 partition. The

subpartition template becomes unusable and we can no longer ADD new

partitions unless we remove/reset the template.

```

-- ERROR:  42P16: invalid boundary specification for RANGE partition

ALTER TABLE heteropart ADD PARTITION f VALUES ('F');

```

Therefore, as shown above, the ALTER TABLE ADD PARTITION and ALTER

TABLE SET SUBPARTITION TEMPLATE commands are supported with

best-effort: depending on who is the current FIRST sibling partition,

which is dictated by the present partition bounds, these two

commands may or may not work as the user desired. If a partition

template works now, it may become invalid later.

## Problems

In my opinion there are two problems:

1. The inconsistency of allowing or disallowing zero child partitions.

2. The dependency on the first partition's partition strategy/key with

ALTER TABLE ADD PARTITION and ALTER TABLE SET PARTITION TEMPLATE

If we can fix problem 2, problem 1 will be alleviated.

## Proposals

### Option 1: Do nothing, add tests and document this

Obviously, this requires the least amount of work. However,

SUBPARTITION TEMPLATE is a nice GPDB feature that allows users to

define and store a uniform subpartition structure in one go, so that

users can re-use it in a future ALTER TABLE ADD PARTITION command.

Having these two commands implemented with a best-effort approach is

really not our best effort.

### Option 2: Specify SUBPARTITION BY as part of the subpartition

template

#### ALTER TABLE SET PARTITION TEMPLATE

Currently we specify subpartition template like this[2]:

```

SET SUBPARTITION TEMPLATE (subpartition_spec)

```

I propose we update/extend the syntax to:

```

SET SUBPARTITION TEMPLATE [ SUBPARTITION BY partition_type (column1)] (subpartition_spec)

```

This allows us to also store the PartitionKey info as a new column in

the `gp_partition_template` catalog table. With this information

available as part of the template itself, we will no longer need to

retrieve the first partition when ALTER TABLE ADD PARTITION on a table

who has subpartition template.

***Since this is catalog change, if we want it, we should do it now.***

We could perhaps make the SUBPARTITION BY clause optional for backward

compatibility and still use the first partition's PartitionKey info if

user didn't specify one AND if we have at least one child partition

present, but we will always record the PartitionKey info in the

`gp_partition_template` catalog table.

#### ALTER TABLE ADD PARTITION

- If a subpartition template (with the subpartition's PartitionKey

  info) is present, we should allow ALTER TABLE ADD PARTITION even if

  the parent table has zero children, because we wouldn't need anything

  from the first child.

- If subpartition template is not present, we could perhaps keep

  current behavior and continue to ask from the first partition if

  available.

Questions and/​​or thoughts?

[1] https://github.com/pivotal/gp-gpdb-historical/commit/01f9a0c799ff4c10452ea51b21d0fb5d329e3697

[2] https://gpdb.docs.pivotal.io/6-12/ref_guide/sql_commands/ALTER_TABLE.html

- Alex W

[Ashwin Agrawal]

On Mon, Oct 17, 2022 at 5:27 PM 'Alexandra Wang' via Greenplum Developers <gpdb...@greenplum.org> wrote:

### Option 2: Specify SUBPARTITION BY as part of the subpartition

template

#### ALTER TABLE SET PARTITION TEMPLATE

Currently we specify subpartition template like this[2]:

```

SET SUBPARTITION TEMPLATE (subpartition_spec)

```

I propose we update/extend the syntax to:

```

SET SUBPARTITION TEMPLATE [ SUBPARTITION BY partition_type (column1)] (subpartition_spec)

```

Current syntax provides flexibility to have heterogeneous hierarchy under sub-partition though the subpartition remains the same. With newer syntax we are proposing to provide flexibility to even allow users to change subpartition by keys. Do we fully understand the implications of that?

This allows us to also store the PartitionKey info as a new column in

the `gp_partition_template` catalog table. With this information

available as part of the template itself, we will no longer need to

retrieve the first partition when ALTER TABLE ADD PARTITION on a table

who has subpartition template.

***Since this is catalog change, if we want it, we should do it now.***

We could perhaps make the SUBPARTITION BY clause optional for backward

compatibility and still use the first partition's PartitionKey info if

user didn't specify one AND if we have at least one child partition

present, but we will always record the PartitionKey info in the

`gp_partition_template` catalog table.

Backward compatibility is the key aspect for supporting these legacy GPDB partition syntax, if it needs modification best to force users to use the newer upstream syntax. So, it has to be optional for sure.

What if we store the subpartition by clause (specified as its today) in the catalog without providing any kind of newer syntax to the user? That would just decouple the dependency on child partition and template would be self contained without any user side syntax change? Just in case in future need arises to modify the subpartition by as well then we visit adding newer syntax.

--

Ashwin Agrawal (VMware)

[Ashwin Agrawal]

On Mon, Oct 17, 2022 at 5:27 PM 'Alexandra Wang' via Greenplum Developers <gpdb...@greenplum.org> wrote:

## Problems

In my opinion there are two problems:

1. The inconsistency of allowing or disallowing zero child partitions.

2. The dependency on the first partition's partition strategy/key with

ALTER TABLE ADD PARTITION and ALTER TABLE SET PARTITION TEMPLATE

Also thinking more, how much does one really care about this zero child partition scenario in production or any use case actually. Is it worth the complicated code in normal flow to handle the same? One can just drop root and start over again on failures for it, right (as no data anyways)?

--

Ashwin Agrawal (VMware)

[Alexandra Wang]

Hi Ashwin, 

Thank you so much for responding!

On Tuesday, October 18, 2022 at 10:46:00 AM UTC-7 Ashwin Agrawal wrote:

On Mon, Oct 17, 2022 at 5:27 PM 'Alexandra Wang' via Greenplum Developers <gpdb...@greenplum.org> wrote:

### Option 2: Specify SUBPARTITION BY as part of the subpartition

template

#### ALTER TABLE SET PARTITION TEMPLATE

Currently we specify subpartition template like this[2]:

```

SET SUBPARTITION TEMPLATE (subpartition_spec)

```

I propose we update/extend the syntax to:

```

SET SUBPARTITION TEMPLATE [ SUBPARTITION BY partition_type (column1)] (subpartition_spec)

```

Current syntax provides flexibility to have heterogeneous hierarchy under sub-partition though the subpartition remains the same. With newer syntax we are proposing to provide flexibility to even allow users to change subpartition by keys. Do we fully understand the implications of that?

Yes, I think we fully understand! Even today with the current syntax
we don't/can't uniform the subpartition keys. Using the PG-style
syntax, users of GPDB7 today can anyways create a new child partition
that itself is (sub)partitioned by any (sub)partition key of their
choice. So the proposed syntax would not introduce any surprises in
that front. In fact, neither the current nor the proposed syntax of
SET SUBPARTITION TEMPLATE would change anything about the existing
partitions and subpartitions. The template is only stored in the
gp_partition_template table to provide reference for future ALTER
TABLE ADD PARTITION commands. 

Without a specification of the partition key, the current SET
SUBPARTITION TEMPLATE syntax can be confusing and error-prone.

Consider the following example:

create table foo (a int, b int, c int, d int) partition by range(b);

create table foo_p2 partition of foo for values from (3) to (6) partition by range(c); -- this is the FIRST sibling partition

create table foo_p2_sp1 partition of foo_p2 for values from (5) to (10);

alter table foo set subpartition template (subpartition sp1 start (5) end (10)); -- the partition key is range(c) according to "foo_p2"

create table foo_p1 partition of foo for values from (0) to (3) partition by range(d); -- this now becomes the FIRST sibling partition according to how we sort range partition bounds

create table foo_p1_sp1 partition of foo_p1 for values from (1000) to (2000);

At this point, the subpartition template is still valid, but it's
corresponding partition key has changed from "range(c)" to "range(d)",
which may or may not be the user's intention.

In my opinion, we are currently providing too much
flexibility/uncertainties to the users than what they
need/expect.

 

This allows us to also store the PartitionKey info as a new column in

the `gp_partition_template` catalog table. With this information

available as part of the template itself, we will no longer need to

retrieve the first partition when ALTER TABLE ADD PARTITION on a table

who has subpartition template.

***Since this is catalog change, if we want it, we should do it now.***

We could perhaps make the SUBPARTITION BY clause optional for backward

compatibility and still use the first partition's PartitionKey info if

user didn't specify one AND if we have at least one child partition

present, but we will always record the PartitionKey info in the

`gp_partition_template` catalog table.

Backward compatibility is the key aspect for supporting these legacy GPDB partition syntax, if it needs modification best to force users to use the newer upstream syntax. So, it has to be optional for sure.

I agree with you. We can make the SUBPARTITION BY clause optional and
still use the first partition's partition key as default. If first
partition is not available we continue to ERROR.

 

What if we store the subpartition by clause (specified as its today) in the catalog without providing any kind of newer syntax to the user? That would just decouple the dependency on child partition and template would be self contained without any user side syntax change? Just in case in future need arises to modify the subpartition by as well then we visit adding newer syntax.

I have also thought about the same. However, I think extending the
syntax to have the optional SUBPARTITION BY clause will make pg_dump's
life easier. If we are going to store the subpartition key in the catalog, why not

also provide the ability to dump it?

 

> On Tuesday, October 18, 2022 at 11:02:41 AM UTC-7 Ashwin Agrawal wrote:

>>  On Mon, Oct 17, 2022 at 5:27 PM 'Alexandra Wang' via Greenplum Developers <gpdb...@greenplum.org> wrote:

>> ## Problems

>> In my opinion there are two problems:

>> 1. The inconsistency of allowing or disallowing zero child partitions.

>> 2. The dependency on the first partition's partition strategy/key with

>> ALTER TABLE ADD PARTITION and ALTER TABLE SET PARTITION TEMPLATE

> Also thinking more, how much does one really care about this zero child partition scenario in production or any use case actually.

You are absolutely right, as a user I wouldn't care much about the zero child partition scenario, but I do hope that my subpartition template is doing its job as I expected. So problem 2 is a better selling point for users.

>  Is it worth the complicated code in normal flow to handle the same? One can just drop root and start over again on failures for it, right (as no data anyways)?

In terms of complication of the code, it is actually the opposite.
Getting rid of the inconsistency with regard to zero child will simply
the code. In this commit, we had to add extra checks in multiple
places just in case the subpartition template is no longer up to date.
Majority of tests added in this commit are also for the zero child
case. As much as the user may not care about zero-child scenario, as a
developer I surely hope not to let the zero-child scenario to
introduce a crash.

- Alex

[Adam Lee]

IMO the inconsistency is not a big deal even there might be other inconsistencies in production.

It’s fine for backward compatibility and changing behaviors breaks the compatibility. I take it as “we

only provide new feature or behavior changes with the new syntax”.

 

From: Ashwin Agrawal <ashwi...@gmail.com>
Date: Wednesday, October 19, 2022 at 02:02
To: Alexandra Wang <walex...@vmware.com>
Cc: Greenplum Developers <gpdb...@greenplum.org>
Subject: Re: GPDB7 Partitioning commands: remove guard for "one partition must remain"

⚠ External Email

---

⚠ External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.

[Alexandra Wang]

Hi Adam, Ashwin and  everyone, 

I believe my previous reply to Ashwin didn't make it to people's mailbox. So, I'm posting the link of my previous reply here:

https://groups.google.com/a/greenplum.org/g/gpdb-dev/c/X9OX-N6wI0w/m/lSx7tz4BCwAJ

(I didn't like how outlook format things, so I did "reply all" directly from the google groups, apparently that didn't go well either. If anyone knows why please may I get some help?) 

Alex

[Alexandra Wang]

Hi Adam,

Thank you for your reply!

On Tuesday, October 18, 2022 at 7:14:21 PM UTC-7 Adam Lee wrote:

> IMO the inconsistency is not a big deal even there might be other inconsistencies in production.

> It’s fine for backward compatibility and changing behaviors breaks the compatibility. I take it as “we

> only provide new feature or behavior changes with the new syntax”.

Yes, I agree with you that the inconsistency between different syntax

may not be a big deal from the user's perspective, and backward

compatibility is necessary in general. However, I don't think this

particular inconsistent behavior is the kind of backward compatibility

that is worth maintaining.

In GPDB6, there are two commands that ensure no-zero-child: CREATE

TABLE and ALTER TABLE DROP PARTITION. If a CREATE TABLE PARTITION BY

statement didn't specify any child partition, the parser would throw a

syntax error right away. We've been disallowing zero-child since the

beginning of the legacy GPDB's partitioning implementation, simply

because we thought a zero-child partitioned table is useless. However,

today if we remove this restriction in GPDB6, things will work just

fine.

In GPDB7, the upstream syntax allows CREATE TABLE to not specify any

child partition and we accepted that. We continued to disallow

zero-child for ALTER TABLE DROP PARTITION. In addition, we also added

the zero-child restriction to ALTER TABLE ADD PARTITION and ALTER

TABLE SET SUBPARTITION TEMPLATE. The reason why we added the new

checks is less of "for backward compatibility", but more of a hack to

make these two commands work. Why? Because these two commands by

themselves don't provide any information about the subpartition

strategy/key. In GPDB6, they don't need to because the partitioning

hierarchy is homogeneous (read: the partition strategy and key for a

given partition level is fixed), we just lookup the subpartition

strategy/key from the catalog using the root table's oid and the

partition level of interest. Whereas in GPDB7, due to its heterogenous

flexibility, we are no longer guaranteed to have a universal

subpartition strategy/key among sibling intermediate partitions, hence

we don't store subpartition strategy/key info in catalog at the

granularity of an entire partition level. So, in GPDB7 we did the hack

for ADD PARTITION and SET SUBPARTITION TEMPLATE to utilize the

partition strategy/key info of the first child partition of the root

table, which may or may not be the same as the rest of the child

partitions, and the first partition itself can change over time.

With all the contexts above said out loud, I realized that I'm also

not that bothered by the inconsistency of the zero-child restriction

itself. What really bothered me is the hack we did for SUBPARTITION

TEMPLATE using the first child partition.

So, I'm going to pivot this discussion to just focus on the aspect of

making SUBPARTITION TEMPLATE work properly in GPDB7.

Below is my updated problem statement and proposal:

Problem:

SUBPARTITION TEMPLATE^[1][2] is a GPDB only feature that provides users

an easy way to create a predefined set of leaf partitions all together

when adding a new intermediate-level partition using ALTER TABLE ADD

PARTITION. However, the current implementation of SUBPARTITION

TEMPLATE and the subsequent ALTER TABLE ADD PARTITION that utilizes

the template in GPDB7 is problematic.

Consider the following workflow:

-- create a subpartition template as part of the root table creation.

-- the subpartition template is designed to work with the subpartition strategy/key: range(k)

create table foo (i int, j int, k int, l int)

partition by range(j)

    subpartition by range (k)

    subpartition template

    (

        subpartition sp1 start (1000) end (2000) every (500)

    )

(  

    partition p1 start(10) end(20), -- FIRST child as of now

    partition p2 start(20) end(30)

);

-- add a new partition to foo utilizing the subpartition template AND

-- the current first child's subpartition key: range(k)

alter table foo add partition p3 start (30) end (40);

-- create a table just like foo and partition this table by range(l)

create table foo_p4 (like foo) partition by range(l) (partition sp1 start (5) end (10));

-- attach this table as a child partition of foo, because of the way 

-- range partition bounds are sorted, this table happens to 

-- then become the FIRST child partition

alter table foo attach partition foo_p4 for values from (0) to (10);

-- add a new partition to foo utilizing the subpartition template AND

-- the current first child's subpartition key: range(l)

alter table foo add partition p5 start (50) end (60);

In the above example, ADD PARTITION at different time with the same

subpartition template ended up using different subpartition keys. I

doubt this is the behavior users would expect, and this would not

happen in GPDB6 and prior versions due to the homogenous nature of the

legacy partitioning implementation.

Proposal:

1. Store subpartition strategy and key information as part of the subpartition template in gp_partition_template table
2. Extend ALTER TABLE SET SUBPARTITION command to include an SUBPARTITION BY clause

Existing syntax^[3]:

SET SUBPARTITION TEMPLATE (subpartition_spec)

Proposed syntax:

SET SUBPARTITION TEMPLATE [ SUBPARTITION BY partition_type (column1)] (subpartition_spec)

For 2, I'd like to hear opinions about whether or not to make the

SUBPARTITION BY clause optional. If we make it optional, we'd have the

syntax "backward compatible", but we'd still have to retrive the first

child partition to infer the subpartition key, which I think is

undesirable.  Given that SUBPARTITION TEMPLATE is usually set as part

of the CREATE TABLE statement, I wonder how often a user would use the

ALTER TABLE SET SUBPARTITION TEMPLATE command to add or modify

subpartition template. Would it be better to just make the

SUBPARTITION BY clause mandatory?

Comments and thoughts?

[1] https://docs.vmware.com/en/VMware-Tanzu-Greenplum/6/greenplum-database/GUID-admin_guide-ddl-ddl-partition.html#defining-multi-level-partitions

[2] https://docs.vmware.com/en/VMware-Tanzu-Greenplum/6/greenplum-database/GUID-admin_guide-ddl-ddl-partition.html#topic85

[3] https://gpdb.docs.pivotal.io/6-12/ref_guide/sql_commands/ALTER_TABLE.html

- Alex

[Ashwin Agrawal]

Thank you for pivoting from zero-child usecase and driving our focus towards more production related scenarios. Acknowledge mixing GPDB legacy syntax and newer upstream syntax might bring more such challenges and we might have to find ways to ease the pain for our customers and avoid confusions in every possible way we can.

Let's clearly document that the subpartition template is purely for convenience during ADD PARTITION and no other command makes use of it like CREATE TABLE..PARTITION OF and such.

I really like the proposed solution to make SUBPARTITION TEMPLATE a self contained entity and completely decouple it from the existing partition structure. GPDB6 syntax implementation never allowed a way to have different subpartition keys across partitions and hence the SET SUBPARTITION TEMPLATE didn't face the need to take as input SUBPARTITION BY. Given the flexibility now it is better to make it a fully self contained command and object in catalog.

I am unclear as of now on how much complexity is being introduced with additional requirement to store and then later use the SUBPARTITION BY in template catalog, we will know more once we get to implement the same. Though I am guessing a lot of existing code will be reused for the same.

For 2, I'd like to hear opinions about whether or not to make the

SUBPARTITION BY clause optional. If we make it optional, we'd have the

syntax "backward compatible", but we'd still have to retrive the first

child partition to infer the subpartition key, which I think is

undesirable.  Given that SUBPARTITION TEMPLATE is usually set as part

of the CREATE TABLE statement, I wonder how often a user would use the

ALTER TABLE SET SUBPARTITION TEMPLATE command to add or modify

subpartition template. Would it be better to just make the

SUBPARTITION BY clause mandatory?

After thinking more and offline discussion with Alex, I am very much supportive of making SUBPARTITION BY mandatory as part of SET SUBPARTITION TEMPLATE. The aim is to make code simple, behavior predictable and easier to understand. These goals can only be accomplished if TEMPLATE always stores SUBPARTITION KEY and no case exists to rely on first child partition. Yes, with that decision users will have to update places using SET SUBPARTITION TEMPLATE, though that change is simple to do and mainly as Alex stated not expecting to be present in too many scripts. General form to specify and populate SUBPARTITION TEMPLATE is via CREATE stmt and that's not changing.

--

Ashwin Agrawal (VMware)

[Alexandra Wang]

Hi all,

So, following Ashwin's suggestion, I went ahead and created PR https://github.com/greenplum-db/gpdb/pull/14508. This PR stores the partition key alongside GpPartitionDefinition in gp_partition_template. So now GetGPPartitionTemplate() returns a PartitionSpec (with the subpart filed always being NULL) instead of a GpPartitionDefinition. However, this change does NOT help us get rid of the code which retrieves the first child partition in each level of the partition hierarchy for ALTER TABLE ADD PARTITION. Only after I started writing the code had I realize:

a) If no partition template exists, we have to rely on the first child partition's partitioning structure to create the partitioning structure of the yet to be added partition. Take below three-level partitioned table with no template as an example, ALTER TABLE ADD PARTITION would expect the to be added partition to be also further partitioned with at least one level 2 child.

-- Create a three-level partitioned table with no template as an example

CREATE TABLE notemplate (a int, dropped int, b int, c int, d int)

    DISTRIBUTED RANDOMLY

    PARTITION BY RANGE (b)

        SUBPARTITION BY RANGE (c)

        (

        PARTITION b_low start (0) end (3)

            (

              SUBPARTITION c_low start (1) end (5),

              SUBPARTITION c_hi start (5) end (10),

              DEFAULT SUBPARTITION def

            ),

        PARTITION b_mid start (3) end (6)

            (

              SUBPARTITION c_low start (1) end (5),

              SUBPARTITION c_hi start (5) end (10)

            )

        );

postgres=# ALTER TABLE notemplate ADD PARTITION b_hi START (6) END (9);
ERROR:  no partitions specified at depth 2

b) Even if partition template exists, we'd still rely on the first child partition's partitioning structure to figure out how many levels of partitioning we are expecting for the to be added partition, unless we invent a holistic partition template for the root partition only, rather than a partition template for each level, we can't get rid of the dependency on the first child partition without breaking backward compatibility with 6X.

Given a) and b), the benefit of the PR is very limited, it only helps the scenario where the first partition has changed to a different partition who happen to have the same number of partitioning levels but with different set of partition keys.

In the current PR, the columns that record the partition key are in non-transformed raw format, whereas the column that stores gpPartiitonDefinition is in transformed format. Ideally, we should like to transform all of them. However, in order to transform gpPartitionDefinition in generatePartitions(), we need to open the parent relation because upstream function transformPartitionBound() is called. ALTER TABLE SET SUBPARTITION TEMPLATE also calls generatePartitions() for validation purposes, and the parent relation in this case is the first child partition of the table to be altered. So again, we cannot easily get rid of the first child partition entirely. Alternatively, we could avoid transforming the subpartition template by store storing the raw format of the template, but that seems very ugly too and inefficient. 

So, I now more inclined to discontinue this work and just document the pitfalls of subpartition template if users start using the newer upstream syntax on the same table, because the gain of this PR is very minimal compared to the effort required to keep backward compatibly. Thoughts?

---

From: Ashwin Agrawal <ashwi...@gmail.com>
Sent: Wednesday, October 19, 2022 3:51 PM
To: Alexandra Wang <walex...@vmware.com>
Cc: Adam Lee <ad...@vmware.com>; Greenplum Developers <gpdb...@greenplum.org>

Subject: Re: GPDB7 Partitioning commands: remove guard for "one partition must remain"

 

⚠ External Email

On Wed, Oct 19, 2022 at 11:21 AM Alexandra Wang <walex...@vmware.com> wrote: