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
>From ea30d12e7cf3f0b5858ebd8f003269b992d10f92 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 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
