subject:"\[HACKERS\] Crash when partition column specified twice"

Re: [HACKERS] Crash when partition column specified twice

2017-04-30 Thread Amit Langote

On 2017/04/29 2:53, Robert Haas wrote:
> On Fri, Apr 28, 2017 at 7:23 AM, Beena Emerson  
> 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

Re: [HACKERS] Crash when partition column specified twice

2017-04-28 Thread Robert Haas

On Fri, Apr 28, 2017 at 7:23 AM, Beena Emerson  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

Re: [HACKERS] Crash when partition column specified twice

2017-04-28 Thread Beena Emerson

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  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

Re: [HACKERS] Crash when partition column specified twice

2017-04-27 Thread Amit Langote

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 
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 28ce

[HACKERS] Crash when partition column specified twice

2017-04-26 Thread Amit Langote

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 
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
@@

Re: [HACKERS] Crash when partition column specified twice

Re: [HACKERS] Crash when partition column specified twice

Re: [HACKERS] Crash when partition column specified twice

Re: [HACKERS] Crash when partition column specified twice

[HACKERS] Crash when partition column specified twice

5 matches

Site Navigation

Mail list logo

Footer information