ADBDEV-3174: New GUC to inherit storage options with ALTER TABLE ADD PARTITION #618

RekGRpth · 2023-09-13T10:28:11Z

AO table inherit storage options with ALTER TABLE ADD PARTITION

Add the gp_add_partition_inherits_table_setting GUC for AO tables. Its default
value is false. When the GUC is set to true, the ALTER TABLE ADD PARTITION
query will add new partitions with storage options (appendonly, orientation,
compresstype, compresslevel, blocksize and checksum) inherited from the parent
table (unless these options are explicitly specified in the DDL or subpartition
template).

BenderArenadata · 2023-09-13T11:44:48Z

Allure report https://allure-ee.adsw.io/launch/54185

BenderArenadata · 2023-09-14T06:25:45Z

Allure report https://allure-ee.adsw.io/launch/54272

BenderArenadata · 2023-09-14T12:33:59Z

Allure report https://allure-ee.adsw.io/launch/54329

When a new partitioned table is created, its partitions inherit storage options (appendonly, orientation, compresstype, compresslevel and blocksize) from the parent table. But when adding partitions using the ALTER TABLE ADD PARTITION command, new partitions are added with storage options as specified in gp_default_storage_options (unless these options are explicitly specified in the DDL or subpartition template). This patch adds a new GUC gp_add_partition_inherits_table_setting with the default value false. When set to true, the ALTER TABLE ADD PARTITION command will add new partitions with storage options inherited from the parent table (unless these options are explicitly specified in the DDL or subpartition template).

BenderArenadata · 2023-09-15T05:00:01Z

Allure report https://allure-ee.adsw.io/launch/54369

src/include/utils/sync_guc_name.h

andr-sokolov · 2023-09-25T14:22:02Z

@RekGRpth , what about making changes in the transformAlterTable_all_PartitionStmt function? The function name begins with "transform", so I think it is more suitable place. The patch below is a draft.

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index b95ff4bb70..8018bc58a6 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -4884,6 +4884,24 @@ transformAlterTable_all_PartitionStmt(
                        heap_close(rel, AccessShareLock);
        } /* end if alter */
 
+       if (atc1->subtype == AT_PartAdd)
+       {
+               PartitionElem *pelem = (PartitionElem *) pc->arg1;
+               if (gp_add_partition_inherits_table_setting && pelem->storeAttr == NULL)
+               {
+                       AlterPartitionCmd *storenode = makeNode(AlterPartitionCmd);
+                       if (RelationIsAoRows(pCxt->rel) || RelationIsAoCols(pCxt->rel))
+                       {
+                               storenode->arg1 = (Node *)build_ao_rel_storage_opts(
+                                               list_make1(makeDefElem("appendonly", (Node *) makeString("true"))),
+                                               pCxt->rel);
+                       }
+
+                       storenode->location = -1;
+                       pelem->storeAttr = (Node *)storenode;
+               }
+       }
+
        switch (atc1->subtype)
        {
                case AT_PartAdd:                                /* Add */

RekGRpth · 2023-09-26T03:44:32Z

what about making changes in the transformAlterTable_all_PartitionStmt function? The function name begins with "transform", so I think it is more suitable place.

moved

BenderArenadata · 2023-09-26T04:14:32Z

Allure report https://allure-ee.adsw.io/launch/55091

BenderArenadata · 2023-09-26T04:44:27Z

Allure report https://allure-ee.adsw.io/launch/55093

andr-sokolov · 2023-10-10T08:32:29Z

@RekGRpth In the current version after this queries

SET gp_add_partition_inherits_table_setting = on;

CREATE TABLE ao_alter_add_part(a int, b int) WITH (appendonly=true, orientation=row, compresstype=zstd) DISTRIBUTED BY (a) PARTITION BY RANGE (b)
  (PARTITION "10" START (1) INCLUSIVE END (10) EXCLUSIVE,
   PARTITION "20" START (11) INCLUSIVE END (20) EXCLUSIVE);

ALTER TABLE ao_alter_add_part ADD PARTITION "30" START (20) INCLUSIVE END (30) EXCLUSIVE;

the added partition has more options than the table and these options are arranged in a different order

postgres=# \d+ ao_alter_add_part*
            Append-Only Table "public.ao_alter_add_part"
 Column |  Type   | Modifiers | Storage | Stats target | Description 
--------+---------+-----------+---------+--------------+-------------
 a      | integer |           | plain   |              | 
 b      | integer |           | plain   |              | 
Compression Type: zstd
Compression Level: 1
Block Size: 32768
Checksum: t
Child tables: ao_alter_add_part_1_prt_10,
              ao_alter_add_part_1_prt_20,
              ao_alter_add_part_1_prt_30
Distributed by: (a)
Partition by: (b)
Options: appendonly=true, orientation=row, compresstype=zstd

        Append-Only Table "public.ao_alter_add_part_1_prt_10"
 Column |  Type   | Modifiers | Storage | Stats target | Description 
--------+---------+-----------+---------+--------------+-------------
 a      | integer |           | plain   |              | 
 b      | integer |           | plain   |              | 
Compression Type: zstd
Compression Level: 1
Block Size: 32768
Checksum: t
Check constraints:
    "ao_alter_add_part_1_prt_10_check" CHECK (b >= 1 AND b < 10)
Inherits: ao_alter_add_part
Distributed by: (a)
Options: appendonly=true, orientation=row, compresstype=zstd

        Append-Only Table "public.ao_alter_add_part_1_prt_20"
 Column |  Type   | Modifiers | Storage | Stats target | Description 
--------+---------+-----------+---------+--------------+-------------
 a      | integer |           | plain   |              | 
 b      | integer |           | plain   |              | 
Compression Type: zstd
Compression Level: 1
Block Size: 32768
Checksum: t
Check constraints:
    "ao_alter_add_part_1_prt_20_check" CHECK (b >= 11 AND b < 20)
Inherits: ao_alter_add_part
Distributed by: (a)
Options: appendonly=true, orientation=row, compresstype=zstd

        Append-Only Table "public.ao_alter_add_part_1_prt_30"
 Column |  Type   | Modifiers | Storage | Stats target | Description 
--------+---------+-----------+---------+--------------+-------------
 a      | integer |           | plain   |              | 
 b      | integer |           | plain   |              | 
Compression Type: zstd
Compression Level: 1
Block Size: 32768
Checksum: t
Check constraints:
    "ao_alter_add_part_1_prt_30_check" CHECK (b >= 20 AND b < 30)
Inherits: ao_alter_add_part
Distributed by: (a)
Options: appendonly=true, blocksize=32768, compresslevel=1, checksum=true, compresstype=zstd, orientation=row

postgres=#

What about this patch to fix the problem?

diff --git a/src/backend/cdb/cdbpartition.c b/src/backend/cdb/cdbpartition.c
index fc9fec0993..7bac71ddcb 100644
--- a/src/backend/cdb/cdbpartition.c
+++ b/src/backend/cdb/cdbpartition.c
@@ -6773,6 +6773,8 @@ atpxPartAddList(Relation rel,
 
 	if (pelem->storeAttr)
 		ct->options = (List *) ((AlterPartitionCmd *) pelem->storeAttr)->arg1;
+	else if (gp_add_partition_inherits_table_setting)
+		ct->options = reloptions_list(RelationGetRelid(rel));
 
 	ct->tableElts = list_concat(ct->tableElts, list_copy(colencs));
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 680516a758..f5d669dd80 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14447,7 +14447,7 @@ drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid, bool is_partitio
 /*
  * deparse pg_class.reloptions into a list.
  */
-static List *
+List *
 reloptions_list(Oid relid)
 {
 	Datum		reloptions;
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 6adee26dfd..be01f932b3 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -137,4 +137,5 @@ extern void RangeVarCallbackOwnsRelation(const RangeVar *relation,
 							 Oid relId, Oid oldRelId, void *noCatalogs);
 
 extern List * rel_get_column_encodings(Relation rel);
+extern List * reloptions_list(Oid relid);
 #endif   /* TABLECMDS_H */

RekGRpth · 2023-10-10T12:19:46Z

What about this patch to fix the problem?

Applied.

BenderArenadata · 2023-10-10T12:43:06Z

Allure report https://allure-ee.adsw.io/launch/56366

BenderArenadata · 2023-10-10T13:20:38Z

Allure report https://allure-ee.adsw.io/launch/56370

BenderArenadata · 2023-10-10T13:32:55Z

Failed job Resource group isolation tests on x86_64: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/696864

BenderArenadata · 2023-11-07T06:13:32Z

Allure report https://allure-ee.adsw.io/launch/57991

BenderArenadata · 2023-11-07T06:25:07Z

Failed job Resource group isolation tests on x86_64: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/765914

BenderArenadata · 2023-11-07T06:30:39Z

Failed job Resource group isolation tests on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/765915

RekGRpth · 2024-10-17T03:23:32Z

bender build

BenderArenadata · 2024-10-17T03:52:54Z

Allure report https://allure.adsw.io/launch/83263

src/backend/cdb/cdbpartition.c

BenderArenadata · 2024-10-21T08:41:45Z

Allure report https://allure.adsw.io/launch/83532

src/test/regress/sql/alter_table_ao.sql

src/backend/commands/tablecmds.c

BenderArenadata · 2024-10-29T05:00:11Z

Allure report https://allure.adsw.io/launch/84210

BenderArenadata · 2024-10-29T06:16:42Z

Allure report https://allure.adsw.io/launch/84214

BenderArenadata · 2024-10-29T07:08:42Z

Allure report https://allure.adsw.io/launch/84218

src/backend/access/common/reloptions_gp.c

BenderArenadata · 2024-10-29T08:15:55Z

Allure report https://allure.adsw.io/launch/84227

BenderArenadata · 2024-10-29T08:58:42Z

Allure report https://allure.adsw.io/launch/84235

RekGRpth changed the title ~~ADBDEV-3174: adding gp_add_partition_inherits_table_setting GUC~~ ADBDEV-3174: New GUC for inheriting storage options with ALTER TABLE ADD PARTITION Sep 14, 2023

RekGRpth changed the title ~~ADBDEV-3174: New GUC for inheriting storage options with ALTER TABLE ADD PARTITION~~ ADBDEV-3174: New GUC to inherit storage options with ALTER TABLE ADD PARTITION Sep 14, 2023

RekGRpth force-pushed the ADBDEV-3174 branch from e3a6530 to d0502bb Compare September 15, 2023 04:20

RekGRpth marked this pull request as ready for review September 15, 2023 04:46

andr-sokolov self-requested a review September 15, 2023 09:21

andr-sokolov reviewed Sep 18, 2023

View reviewed changes

src/include/utils/sync_guc_name.h Show resolved Hide resolved

RekGRpth added 3 commits September 26, 2023 08:32

move logic from exec to plan and fix tests

4194c7e

apply pgindent to changes

350bee9

Merge branch 'adb-6.x-dev' into ADBDEV-3174

2bb1a99

RekGRpth added 2 commits October 10, 2023 17:17

move logic

6ef7fdd

Merge branch 'adb-6.x-dev' into ADBDEV-3174

98598c6

RekGRpth added 2 commits November 7, 2023 10:08

fix

2c33cf4

Merge branch 'adb-6.x-dev' into ADBDEV-3174

4065a5c

RekGRpth added 3 commits October 16, 2024 11:51

save and restore

e40f604

try-catch

b688923

optimize

b1da2e1

Merge branch 'adb-6.x-dev' into ADBDEV-3174

d100004

andr-sokolov reviewed Oct 18, 2024

View reviewed changes

src/backend/cdb/cdbpartition.c Outdated Show resolved Hide resolved

RekGRpth added 2 commits October 21, 2024 12:23

move condition

0dcba65

AO

6c807a3