Crash when partition column specified twice

Started by Amit Langoteover 8 years ago5 messages

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

1 attachment(s)

Noticed that a crash occurs if a column is specified twice when creating a
partition:

create table p (a int) partition by list (a);

-- crashes
create table p1 partition of parent (
a not null,
a default 1
) for values in (1);

The logic in MergeAttributes() that merged partition column options with
those of the parent didn't properly check for column being specified twice
and instead tried to delete the same ColumnDef from a list twice, causing
the crash.

Attached fixes that.

Added to the open items list.

Thanks,
Amit

Attachments:

0001-Fix-crash-when-partition-column-specified-twice.patchtext/x-diff; name=0001-Fix-crash-when-partition-column-specified-twice.patchDownload

From 8b1c2ea63c22f4b5180244a41350c2391caffda3 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 27 Apr 2017 11:22:55 +0900
Subject: [PATCH] Fix crash when partition column specified twice

---
 src/backend/commands/sequence.c            |  1 +
 src/backend/commands/tablecmds.c           | 20 +++++++++++++++-----
 src/backend/nodes/copyfuncs.c              |  1 +
 src/backend/nodes/equalfuncs.c             |  1 +
 src/backend/nodes/makefuncs.c              |  1 +
 src/backend/nodes/outfuncs.c               |  1 +
 src/backend/parser/gram.y                  |  5 +++++
 src/backend/parser/parse_utilcmd.c         |  2 ++
 src/include/nodes/parsenodes.h             |  1 +
 src/test/regress/expected/create_table.out |  8 ++++++++
 src/test/regress/sql/create_table.sql      |  9 +++++++++
 11 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index ad28225b36..acd4e359bb 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -167,6 +167,7 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
 		coldef->is_local = true;
 		coldef->is_not_null = true;
 		coldef->is_from_type = false;
+		coldef->is_from_parent = false;
 		coldef->storage = 0;
 		coldef->raw_default = NULL;
 		coldef->cooked_default = NULL;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a35713096d..4df17c0efc 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1919,6 +1919,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 				def->is_local = false;
 				def->is_not_null = attribute->attnotnull;
 				def->is_from_type = false;
+				def->is_from_parent = true;
 				def->storage = attribute->attstorage;
 				def->raw_default = NULL;
 				def->cooked_default = NULL;
@@ -2206,11 +2207,20 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 					 * merge the column options into the column from the
 					 * parent
 					 */
-					coldef->is_not_null = restdef->is_not_null;
-					coldef->raw_default = restdef->raw_default;
-					coldef->cooked_default = restdef->cooked_default;
-					coldef->constraints = restdef->constraints;
-					list_delete_cell(schema, rest, prev);
+					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;
+						coldef->constraints = restdef->constraints;
+						coldef->is_from_parent = false;
+						list_delete_cell(schema, rest, prev);
+					}
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_DUPLICATE_COLUMN),
+								 errmsg("column \"%s\" specified more than once",
+										coldef->colname)));
 				}
 				prev = rest;
 				rest = next;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 00a0fed23d..8fb872d288 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2804,6 +2804,7 @@ _copyColumnDef(const ColumnDef *from)
 	COPY_SCALAR_FIELD(is_local);
 	COPY_SCALAR_FIELD(is_not_null);
 	COPY_SCALAR_FIELD(is_from_type);
+	COPY_SCALAR_FIELD(is_from_parent);
 	COPY_SCALAR_FIELD(storage);
 	COPY_NODE_FIELD(raw_default);
 	COPY_NODE_FIELD(cooked_default);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 46573ae767..21dfbb0d75 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2540,6 +2540,7 @@ _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
 	COMPARE_SCALAR_FIELD(is_local);
 	COMPARE_SCALAR_FIELD(is_not_null);
 	COMPARE_SCALAR_FIELD(is_from_type);
+	COMPARE_SCALAR_FIELD(is_from_parent);
 	COMPARE_SCALAR_FIELD(storage);
 	COMPARE_NODE_FIELD(raw_default);
 	COMPARE_NODE_FIELD(cooked_default);
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index e88d82f3b0..f5fde1533f 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -494,6 +494,7 @@ makeColumnDef(const char *colname, Oid typeOid, int32 typmod, Oid collOid)
 	n->is_local = true;
 	n->is_not_null = false;
 	n->is_from_type = false;
+	n->is_from_parent = false;
 	n->storage = 0;
 	n->raw_default = NULL;
 	n->cooked_default = NULL;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 28cef85579..05a78b32b7 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2766,6 +2766,7 @@ _outColumnDef(StringInfo str, const ColumnDef *node)
 	WRITE_BOOL_FIELD(is_local);
 	WRITE_BOOL_FIELD(is_not_null);
 	WRITE_BOOL_FIELD(is_from_type);
+	WRITE_BOOL_FIELD(is_from_parent);
 	WRITE_CHAR_FIELD(storage);
 	WRITE_NODE_FIELD(raw_default);
 	WRITE_NODE_FIELD(cooked_default);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 89d2836c49..54b3359e26 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3273,6 +3273,7 @@ PartitionElement:
 				n->is_local = true;
 				n->is_not_null = false;
 				n->is_from_type = false;
+				n->is_from_parent = false;
 				n->storage = 0;
 				n->raw_default = NULL;
 				n->cooked_default = NULL;
@@ -3293,6 +3294,7 @@ columnDef:	ColId Typename create_generic_options ColQualList
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
@@ -3314,6 +3316,7 @@ columnOptions:	ColId WITH OPTIONS ColQualList
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
@@ -11910,6 +11913,8 @@ TableFuncElement:	ColId Typename opt_collate_clause
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index e5461944e2..e187409f6f 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -983,6 +983,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		def->is_local = true;
 		def->is_not_null = attribute->attnotnull;
 		def->is_from_type = false;
+		def->is_from_parent = false;
 		def->storage = 0;
 		def->raw_default = NULL;
 		def->cooked_default = NULL;
@@ -1221,6 +1222,7 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
 		n->is_local = true;
 		n->is_not_null = false;
 		n->is_from_type = true;
+		n->is_from_parent = false;
 		n->storage = 0;
 		n->raw_default = NULL;
 		n->cooked_default = NULL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9f57388781..e1d454a07d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -643,6 +643,7 @@ typedef struct ColumnDef
 	bool		is_local;		/* column has local (non-inherited) def'n */
 	bool		is_not_null;	/* NOT NULL constraint specified? */
 	bool		is_from_type;	/* column definition came from table type */
+	bool		is_from_parent;	/* column def came from partition parent */
 	char		storage;		/* attstorage setting, or 0 for default */
 	Node	   *raw_default;	/* default value (untransformed parse tree) */
 	Node	   *cooked_default; /* default value (transformed expr tree) */
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index b6c75d2e81..ecc561f46d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -609,6 +609,14 @@ SELECT attname, attislocal, attinhcount FROM pg_attribute
 (2 rows)
 
 -- able to specify column default, column constraint, and table constraint
+-- first check the "column specified more than once" error
+CREATE TABLE part_b PARTITION OF parted (
+	b NOT NULL,
+	b DEFAULT 1,
+	b CHECK (b >= 0),
+	CONSTRAINT check_a CHECK (length(a) > 0)
+) FOR VALUES IN ('b');
+ERROR:  column "b" specified more than once
 CREATE TABLE part_b PARTITION OF parted (
 	b NOT NULL DEFAULT 1 CHECK (b >= 0),
 	CONSTRAINT check_a CHECK (length(a) > 0)
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index b00d9e87b8..593880774a 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -569,6 +569,15 @@ SELECT attname, attislocal, attinhcount FROM pg_attribute
   ORDER BY attnum;
 
 -- able to specify column default, column constraint, and table constraint
+
+-- first check the "column specified more than once" error
+CREATE TABLE part_b PARTITION OF parted (
+	b NOT NULL,
+	b DEFAULT 1,
+	b CHECK (b >= 0),
+	CONSTRAINT check_a CHECK (length(a) > 0)
+) FOR VALUES IN ('b');
+
 CREATE TABLE part_b PARTITION OF parted (
 	b NOT NULL DEFAULT 1 CHECK (b >= 0),
 	CONSTRAINT check_a CHECK (length(a) > 0)
-- 
2.11.0

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Amit Langote (#1)

1 attachment(s)

Re: Crash when partition column specified twice

On 2017/04/27 12:36, Amit Langote wrote:

Noticed that a crash occurs if a column is specified twice when creating a
partition:

create table p (a int) partition by list (a);

-- crashes
create table p1 partition of parent (
a not null,
a default 1
) for values in (1);

The logic in MergeAttributes() that merged partition column options with
those of the parent didn't properly check for column being specified twice
and instead tried to delete the same ColumnDef from a list twice, causing
the crash.

Attached fixes that.

Patch rebased, because of a conflict with b9a3ef55b2.

Thanks,
Amit

Attachments:

0001-Fix-crash-when-partition-column-specified-twice.patchtext/x-diff; name=0001-Fix-crash-when-partition-column-specified-twice.patchDownload

From ea30d12e7cf3f0b5858ebd8f003269b992d10f92 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 27 Apr 2017 11:22:55 +0900
Subject: [PATCH] Fix crash when partition column specified twice

---
 src/backend/commands/sequence.c            |  1 +
 src/backend/commands/tablecmds.c           | 20 +++++++++++++++-----
 src/backend/nodes/copyfuncs.c              |  1 +
 src/backend/nodes/equalfuncs.c             |  1 +
 src/backend/nodes/makefuncs.c              |  1 +
 src/backend/nodes/outfuncs.c               |  1 +
 src/backend/parser/gram.y                  |  5 +++++
 src/backend/parser/parse_utilcmd.c         |  2 ++
 src/include/nodes/parsenodes.h             |  1 +
 src/test/regress/expected/create_table.out |  8 ++++++++
 src/test/regress/sql/create_table.sql      |  9 +++++++++
 11 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index ad28225b36..acd4e359bb 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -167,6 +167,7 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
 		coldef->is_local = true;
 		coldef->is_not_null = true;
 		coldef->is_from_type = false;
+		coldef->is_from_parent = false;
 		coldef->storage = 0;
 		coldef->raw_default = NULL;
 		coldef->cooked_default = NULL;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a35713096d..4df17c0efc 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1919,6 +1919,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 				def->is_local = false;
 				def->is_not_null = attribute->attnotnull;
 				def->is_from_type = false;
+				def->is_from_parent = true;
 				def->storage = attribute->attstorage;
 				def->raw_default = NULL;
 				def->cooked_default = NULL;
@@ -2206,11 +2207,20 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 					 * merge the column options into the column from the
 					 * parent
 					 */
-					coldef->is_not_null = restdef->is_not_null;
-					coldef->raw_default = restdef->raw_default;
-					coldef->cooked_default = restdef->cooked_default;
-					coldef->constraints = restdef->constraints;
-					list_delete_cell(schema, rest, prev);
+					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;
+						coldef->constraints = restdef->constraints;
+						coldef->is_from_parent = false;
+						list_delete_cell(schema, rest, prev);
+					}
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_DUPLICATE_COLUMN),
+								 errmsg("column \"%s\" specified more than once",
+										coldef->colname)));
 				}
 				prev = rest;
 				rest = next;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 00a0fed23d..8fb872d288 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2804,6 +2804,7 @@ _copyColumnDef(const ColumnDef *from)
 	COPY_SCALAR_FIELD(is_local);
 	COPY_SCALAR_FIELD(is_not_null);
 	COPY_SCALAR_FIELD(is_from_type);
+	COPY_SCALAR_FIELD(is_from_parent);
 	COPY_SCALAR_FIELD(storage);
 	COPY_NODE_FIELD(raw_default);
 	COPY_NODE_FIELD(cooked_default);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 46573ae767..21dfbb0d75 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2540,6 +2540,7 @@ _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
 	COMPARE_SCALAR_FIELD(is_local);
 	COMPARE_SCALAR_FIELD(is_not_null);
 	COMPARE_SCALAR_FIELD(is_from_type);
+	COMPARE_SCALAR_FIELD(is_from_parent);
 	COMPARE_SCALAR_FIELD(storage);
 	COMPARE_NODE_FIELD(raw_default);
 	COMPARE_NODE_FIELD(cooked_default);
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index e88d82f3b0..f5fde1533f 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -494,6 +494,7 @@ makeColumnDef(const char *colname, Oid typeOid, int32 typmod, Oid collOid)
 	n->is_local = true;
 	n->is_not_null = false;
 	n->is_from_type = false;
+	n->is_from_parent = false;
 	n->storage = 0;
 	n->raw_default = NULL;
 	n->cooked_default = NULL;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 28cef85579..05a78b32b7 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2766,6 +2766,7 @@ _outColumnDef(StringInfo str, const ColumnDef *node)
 	WRITE_BOOL_FIELD(is_local);
 	WRITE_BOOL_FIELD(is_not_null);
 	WRITE_BOOL_FIELD(is_from_type);
+	WRITE_BOOL_FIELD(is_from_parent);
 	WRITE_CHAR_FIELD(storage);
 	WRITE_NODE_FIELD(raw_default);
 	WRITE_NODE_FIELD(cooked_default);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 21cdc7c7da..c4f40a3af2 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3253,6 +3253,7 @@ columnDef:	ColId Typename create_generic_options ColQualList
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
@@ -3274,6 +3275,7 @@ columnOptions:	ColId ColQualList
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
@@ -3292,6 +3294,7 @@ columnOptions:	ColId ColQualList
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
@@ -11888,6 +11891,8 @@ TableFuncElement:	ColId Typename opt_collate_clause
 					n->is_local = true;
 					n->is_not_null = false;
 					n->is_from_type = false;
+					n->is_from_type = false;
+					n->is_from_parent = false;
 					n->storage = 0;
 					n->raw_default = NULL;
 					n->cooked_default = NULL;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index e5461944e2..e187409f6f 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -983,6 +983,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		def->is_local = true;
 		def->is_not_null = attribute->attnotnull;
 		def->is_from_type = false;
+		def->is_from_parent = false;
 		def->storage = 0;
 		def->raw_default = NULL;
 		def->cooked_default = NULL;
@@ -1221,6 +1222,7 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
 		n->is_local = true;
 		n->is_not_null = false;
 		n->is_from_type = true;
+		n->is_from_parent = false;
 		n->storage = 0;
 		n->raw_default = NULL;
 		n->cooked_default = NULL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9f57388781..e1d454a07d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -643,6 +643,7 @@ typedef struct ColumnDef
 	bool		is_local;		/* column has local (non-inherited) def'n */
 	bool		is_not_null;	/* NOT NULL constraint specified? */
 	bool		is_from_type;	/* column definition came from table type */
+	bool		is_from_parent;	/* column def came from partition parent */
 	char		storage;		/* attstorage setting, or 0 for default */
 	Node	   *raw_default;	/* default value (untransformed parse tree) */
 	Node	   *cooked_default; /* default value (transformed expr tree) */
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 3f94250df2..dda0d7ee5d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -609,6 +609,14 @@ SELECT attname, attislocal, attinhcount FROM pg_attribute
 (2 rows)
 
 -- able to specify column default, column constraint, and table constraint
+-- first check the "column specified more than once" error
+CREATE TABLE part_b PARTITION OF parted (
+	b NOT NULL,
+	b DEFAULT 1,
+	b CHECK (b >= 0),
+	CONSTRAINT check_a CHECK (length(a) > 0)
+) FOR VALUES IN ('b');
+ERROR:  column "b" specified more than once
 CREATE TABLE part_b PARTITION OF parted (
 	b NOT NULL DEFAULT 1 CHECK (b >= 0),
 	CONSTRAINT check_a CHECK (length(a) > 0)
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f08942f7d2..caf5ddb58b 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -569,6 +569,15 @@ SELECT attname, attislocal, attinhcount FROM pg_attribute
   ORDER BY attnum;
 
 -- able to specify column default, column constraint, and table constraint
+
+-- first check the "column specified more than once" error
+CREATE TABLE part_b PARTITION OF parted (
+	b NOT NULL,
+	b DEFAULT 1,
+	b CHECK (b >= 0),
+	CONSTRAINT check_a CHECK (length(a) > 0)
+) FOR VALUES IN ('b');
+
 CREATE TABLE part_b PARTITION OF parted (
 	b NOT NULL DEFAULT 1 CHECK (b >= 0),
 	CONSTRAINT check_a CHECK (length(a) > 0)
-- 
2.11.0

Beena Emerson

memissemerson@gmail.com

over 8 years ago

In reply to: Amit Langote (#2)

Re: Crash when partition column specified twice

Hello Amit,

The extra n->is_from_type = false; seems to be added by mistake?

@@ -11888,6 +11891,8 @@ TableFuncElement:   ColId Typename
opt_collate_clause
                    n->is_local = true;
                    n->is_not_null = false;
                    n->is_from_type = false;
+                   n->is_from_type = false;
+                   n->is_from_parent = false;
                    n->storage = 0;
                    n->raw_default = NULL;
                    n->cooked_default = NULL;

On Fri, Apr 28, 2017 at 6:08 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp

wrote:

On 2017/04/27 12:36, Amit Langote wrote:

Noticed that a crash occurs if a column is specified twice when creating

a

partition:

create table p (a int) partition by list (a);

-- crashes
create table p1 partition of parent (
a not null,
a default 1
) for values in (1);

The logic in MergeAttributes() that merged partition column options with
those of the parent didn't properly check for column being specified

twice

and instead tried to delete the same ColumnDef from a list twice, causing
the crash.

Attached fixes that.

Patch rebased, because of a conflict with b9a3ef55b2.

Thanks,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas

robertmhaas@gmail.com

over 8 years ago

In reply to: Beena Emerson (#3)

Re: Crash when partition column specified twice

On Fri, Apr 28, 2017 at 7:23 AM, Beena Emerson <memissemerson@gmail.com> wrote:

Hello Amit,

The extra n->is_from_type = false; seems to be added by mistake?

@@ -11888,6 +11891,8 @@ TableFuncElement:   ColId Typename
opt_collate_clause
n->is_local = true;
n->is_not_null = false;
n->is_from_type = false;
+                   n->is_from_type = false;
+                   n->is_from_parent = false;
n->storage = 0;
n->raw_default = NULL;
n->cooked_default = NULL;

Good catch. Committed after fixing that issue.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 8 years ago

In reply to: Robert Haas (#4)

Re: Crash when partition column specified twice

On 2017/04/29 2:53, Robert Haas wrote:

On Fri, Apr 28, 2017 at 7:23 AM, Beena Emerson <memissemerson@gmail.com> wrote:
Hello Amit,

The extra n->is_from_type = false; seems to be added by mistake?
@@ -11888,6 +11891,8 @@ TableFuncElement:   ColId Typename
opt_collate_clause
n->is_local = true;
n->is_not_null = false;
n->is_from_type = false;
+                   n->is_from_type = false;
+                   n->is_from_parent = false;
n->storage = 0;
n->raw_default = NULL;
n->cooked_default = NULL;
Good catch. Committed after fixing that issue.

Thanks both.

Regards,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers