I noticed that running ALTER TABLE table_name DETACH PARTITION crashes, if
table_name is not a partitioned table. That's because of an Assert in
ATExecDetachPartition(). We really should error out much sooner in this
case, IOW during transformAlterTableStmt(), as is done in the case of
ATTACH PARTITION.
Attached patch fixes that.
Thanks,
Amit
>From d74189febc751302f13009d69bebd763c8a23f58 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 13 Feb 2017 16:10:17 +0900
Subject: [PATCH] Unbreak ALTER TABLE DETACH PARTITION
DETACH PARTITION should throw error much sooner if the specified
(parent) table is not a partitioned table. In case of ATTACH
PARTITION that's done during transformAlterTableStmt. Do the same
for DETACH PARTITION. Instead of inventing a transformDetachPartition,
rename the existing transformAttachPartition to transformPartitionCmd
and have it serve both cases.
---
src/backend/parser/parse_utilcmd.c | 29 +++++++++++++++++++----------
src/test/regress/expected/alter_table.out | 5 +++++
src/test/regress/sql/alter_table.sql | 5 +++++
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 0f78abaae2..efbd205b63 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -133,7 +133,7 @@ static void transformConstraintAttrs(CreateStmtContext *cxt,
List *constraintList);
static void transformColumnType(CreateStmtContext *cxt, ColumnDef *column);
static void setSchemaName(char *context_schema, char **stmt_schema_name);
-static void transformAttachPartition(CreateStmtContext *cxt, PartitionCmd *cmd);
+static void transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd);
/*
@@ -2654,13 +2654,18 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
}
case AT_AttachPartition:
+ case AT_DetachPartition:
{
PartitionCmd *partcmd = (PartitionCmd *) cmd->def;
- transformAttachPartition(&cxt, partcmd);
+ transformPartitionCmd(&cxt, partcmd);
- /* assign transformed values */
- partcmd->bound = cxt.partbound;
+ /*
+ * Assign transformed value of the partition bound, if
+ * any.
+ */
+ if (cxt.partbound != NULL)
+ partcmd->bound = cxt.partbound;
}
newcmds = lappend(newcmds, cmd);
@@ -3032,11 +3037,14 @@ setSchemaName(char *context_schema, char **stmt_schema_name)
}
/*
- * transformAttachPartition
- * Analyze ATTACH PARTITION ... FOR VALUES ...
+ * transformPartitionCmd
+ * Analyze the ATTACH/DETACH PARTITION command
+ *
+ * In case of the ATTACH PARTITION command, cxt->partbound is set to the
+ * transformed value of cmd->bound.
*/
static void
-transformAttachPartition(CreateStmtContext *cxt, PartitionCmd *cmd)
+transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd)
{
Relation parentRel = cxt->rel;
@@ -3050,10 +3058,11 @@ transformAttachPartition(CreateStmtContext *cxt, PartitionCmd *cmd)
errmsg("\"%s\" is not partitioned",
RelationGetRelationName(parentRel))));
- /* transform the values */
+ /* transform the partition bound, if any */
Assert(RelationGetPartitionKey(parentRel) != NULL);
- cxt->partbound = transformPartitionBound(cxt->pstate, parentRel,
- cmd->bound);
+ if (cmd->bound != NULL)
+ cxt->partbound = transformPartitionBound(cxt->pstate, parentRel,
+ cmd->bound);
}
/*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index d8e7b61294..33f820a245 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3255,6 +3255,11 @@ DETAIL: "list_parted2" is already a child of "list_parted2".
--
-- DETACH PARTITION
--
+-- check that the table is partitioned at all
+CREATE TABLE regular_table (a int);
+ALTER TABLE regular_table DETACH PARTITION any_name;
+ERROR: "regular_table" is not partitioned
+DROP TABLE regular_table;
-- check that the partition being detached exists at all
ALTER TABLE list_parted2 DETACH PARTITION part_4;
ERROR: relation "part_4" does not exist
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 1f551ec53c..a2baf0f2a0 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2134,6 +2134,11 @@ ALTER TABLE list_parted2 ATTACH PARTITION list_parted2 FOR VALUES IN (0);
-- DETACH PARTITION
--
+-- check that the table is partitioned at all
+CREATE TABLE regular_table (a int);
+ALTER TABLE regular_table DETACH PARTITION any_name;
+DROP TABLE regular_table;
+
-- check that the partition being detached exists at all
ALTER TABLE list_parted2 DETACH PARTITION part_4;
--
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
