AT detach partition is broken

On Mon, Feb 13, 2017 at 2:30 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

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.

-                    /* assign transformed values */
-                    partcmd->bound = cxt.partbound;
+                    /*
+                     * Assign transformed value of the partition bound, if
+                     * any.
+                     */
+                    if (cxt.partbound != NULL)
+                        partcmd->bound = cxt.partbound;

This hunk isn't really needed, is it? I mean, if cxt.partbound comes
out NULL, then partcmd->bound will be NULL with or without adding an
"if" here, won't it?

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

On 2017/02/15 2:37, Robert Haas wrote:

On Mon, Feb 13, 2017 at 2:30 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

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.

-                    /* assign transformed values */
-                    partcmd->bound = cxt.partbound;
+                    /*
+                     * Assign transformed value of the partition bound, if
+                     * any.
+                     */
+                    if (cxt.partbound != NULL)
+                        partcmd->bound = cxt.partbound;

This hunk isn't really needed, is it? I mean, if cxt.partbound comes
out NULL, then partcmd->bound will be NULL with or without adding an
"if" here, won't it?

You're right. Took this one out (except slightly tweaking the comment) in
the attached updated patch.

Thanks,
Amit

On Wed, Feb 15, 2017 at 7:45 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

You're right. Took this one out (except slightly tweaking the comment) in
the attached updated patch.

Committed after tweaking the other comment in that function.

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