PG11-HEAD-v2-0001-Fix-bug-regarding-partition-column-option-inherit.patch

text/plain

Filename: PG11-HEAD-v2-0001-Fix-bug-regarding-partition-column-option-inherit.patch
Type: text/plain
Part: 1
Message: Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

Patch

Format: format-patch
Series: patch v2-0001
Subject: Fix bug regarding partition column option inheritance
File+
src/backend/commands/tablecmds.c 44 5
src/test/regress/expected/create_table.out 13 0
src/test/regress/sql/create_table.sql 7 0
From 6ab930fa7e8f6f5e548b783451f2ab7d4515909b Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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