From 6ab930fa7e8f6f5e548b783451f2ab7d4515909b 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 7c0cf0d7ee..9955e81d14 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -2394,6 +2394,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) @@ -2421,14 +2430,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 8fdbca1345..5750344e73 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -727,6 +727,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 78944950fe..5a64ecc480 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -659,6 +659,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