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
>From 8b1c2ea63c22f4b5180244a41350c2391caffda3 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
