From 1158793b899bbc1f52e37b2255bd74121a293495 Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 6 Jun 2018 16:46:46 +0900 Subject: [PATCH v2] Fix bug regarding partition column option inheritance It appears that the coding pattern in MergeAttributes causes the NOT NULL constraint and default value of a column from not being properly inherited from the parent's definition. --- src/backend/commands/tablecmds.c | 49 +++++++++++++++++++++++++++--- src/test/regress/expected/create_table.out | 13 ++++++++ src/test/regress/sql/create_table.sql | 7 +++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 81c3845095..b7a6c1792e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -2181,6 +2181,15 @@ MergeAttributes(List *schema, List *supers, char relpersistence, */ if (is_partition && list_length(saved_schema) > 0) { + /* + * Each member in 'saved_schema' contains a ColumnDef containing + * partition-specific options for the column. Below, we merge the + * information from each into the ColumnDef of the same name found in + * the original 'schema' list before deleting it from the list. So, + * once we've finished processing all the columns from the original + * 'schema' list, there shouldn't be any ColumnDefs left that came + * from the 'saved_schema' list. + */ schema = list_concat(schema, saved_schema); foreach(entry, schema) @@ -2208,14 +2217,44 @@ MergeAttributes(List *schema, List *supers, char relpersistence, if (strcmp(coldef->colname, restdef->colname) == 0) { /* - * merge the column options into the column from the - * parent + * Merge the column options specified for the partition + * into the column definition inherited from the parent. */ if (coldef->is_from_parent) { - coldef->is_not_null = restdef->is_not_null; - coldef->raw_default = restdef->raw_default; - coldef->cooked_default = restdef->cooked_default; + /* + * Combine using OR so that the NOT NULL constraint + * in the parent's definition doesn't get lost. + */ + coldef->is_not_null = (coldef->is_not_null || + restdef->is_not_null); + + /* + * If the partition's definition specifies a default, + * it's in restdef->raw_default, which if non-NULL, + * overrides the parent's default that's in + * coldef->cooked_default. + */ + if (restdef->raw_default) + { + coldef->raw_default = restdef->raw_default; + coldef->cooked_default = NULL; + } + /* + * Don't accidentally lose the parent's default by + * overwriting with NULL. + */ + else if (restdef->cooked_default) + { + coldef->raw_default = NULL; + coldef->cooked_default = restdef->cooked_default; + } + + /* + * Parent's constraints, if any, have been saved in + * 'constraints', which are passed back to the caller, + * so it's okay to overwrite the variable like this. + */ coldef->constraints = restdef->constraints; coldef->is_from_parent = false; list_delete_cell(schema, rest, prev); diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 0026aa11dd..4846a6fc98 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -662,6 +662,19 @@ ERROR: column "c" named in partition key does not exist CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR VALUES IN ('c') PARTITION BY RANGE ((b)); -- create a level-2 partition CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); +-- check that NOT NULL and default value are inherited correctly +create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a); +create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (b default 1) for values in (1); +-- note that while b's default is overriden, a's default is preserved +\d parted_notnull_inh_test1 + Table "public.parted_notnull_inh_test1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | 1 + b | integer | | not null | 1 +Partition of: parted_notnull_inh_test FOR VALUES IN (1) + +drop table parted_notnull_inh_test; -- Partition bound in describe output \d+ part_b Table "public.part_b" diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index ad83614137..7442c4f7d3 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -609,6 +609,13 @@ CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR -- create a level-2 partition CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); +-- check that NOT NULL and default value are inherited correctly +create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a); +create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (b default 1) for values in (1); +-- note that while b's default is overriden, a's default is preserved +\d parted_notnull_inh_test1 +drop table parted_notnull_inh_test; + -- Partition bound in describe output \d+ part_b -- 2.11.0