From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: ALTER INDEX fails on partitioned index
Date: Mon, 23 Mar 2020 16:47:04 -0500
Message-ID: <20200323214704.GM2563@telsasoft.com> (raw)
In-Reply-To: <20200227232513.GE29456@telsasoft.com>
References: <20190206173212.GA23770@alvherre.pgsql>
<201901071923.7r2xflxvncly@alvherre.pgsql>
<20191227035157.GD12890@telsasoft.com>
<CA+Tgmobq8_-DS7qDEmMi-4ARP1_0bkgFEjYfiK97L2eXq+Q+nw@mail.gmail.com>
<20200227232513.GE29456@telsasoft.com>
On Thu, Feb 27, 2020 at 09:11:14PM -0300, Alvaro Herrera wrote:
> On 2020-Feb-27, Justin Pryzby wrote:
> > The attached allows CREATE/ALTER to specify reloptions on a partitioned table
> > which are used as defaults for future children.
> >
> > I think that's a desirable behavior, same as for tablespaces. Michael
> > mentioned that ALTER INDEX ONLY doesn't exist, but that's only an issue if
> > ALTER acts recursively, which isn't the case here.
>
> I think ALTER not acting recursively is a bug that we would do well not
> to propagate any further. Enough effort we have to spend trying to fix
> that already. Let's add ALTER .. ONLY if needed.
I was modeling after the behavior for tablespaces, and didn't realize that
non-recursive alter was considered discouraged.
On Thu, Feb 27, 2020 at 05:25:13PM -0600, Justin Pryzby wrote:
> The first patch makes a prettier message, per Robert's suggestion.
Is there any interest in this one ?
> From e5bb363f514d768a4f540d9c82ad5745944b1486 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryzbyj@telsasoft.com>
> Date: Mon, 30 Dec 2019 09:31:03 -0600
> Subject: [PATCH v2 1/2] More specific error message when failing to alter a
> partitioned index..
>
> "..is not an index" is deemed to be unfriendly
>
> https://www.postgresql.org/message-id/CA%2BTgmobq8_-DS7qDEmMi-4ARP1_0bkgFEjYfiK97L2eXq%2BQ%2Bnw%40ma...
> ---
> src/backend/commands/tablecmds.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
> index b7c8d66..1b271af 100644
> --- a/src/backend/commands/tablecmds.c
> +++ b/src/backend/commands/tablecmds.c
> @@ -366,7 +366,7 @@ static void ATRewriteTables(AlterTableStmt *parsetree,
> static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode);
> static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel);
> static void ATSimplePermissions(Relation rel, int allowed_targets);
> -static void ATWrongRelkindError(Relation rel, int allowed_targets);
> +static void ATWrongRelkindError(Relation rel, int allowed_targets, int actual_target);
> static void ATSimpleRecursion(List **wqueue, Relation rel,
> AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode,
> AlterTableUtilityContext *context);
> @@ -5458,7 +5458,7 @@ ATSimplePermissions(Relation rel, int allowed_targets)
>
> /* Wrong target type? */
> if ((actual_target & allowed_targets) == 0)
> - ATWrongRelkindError(rel, allowed_targets);
> + ATWrongRelkindError(rel, allowed_targets, actual_target);
>
> /* Permissions checks */
> if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
> @@ -5479,7 +5479,7 @@ ATSimplePermissions(Relation rel, int allowed_targets)
> * type.
> */
> static void
> -ATWrongRelkindError(Relation rel, int allowed_targets)
> +ATWrongRelkindError(Relation rel, int allowed_targets, int actual_target)
> {
> char *msg;
>
> @@ -5527,9 +5527,20 @@ ATWrongRelkindError(Relation rel, int allowed_targets)
> break;
> }
>
> - ereport(ERROR,
> - (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> - errmsg(msg, RelationGetRelationName(rel))));
> + if (actual_target == ATT_PARTITIONED_INDEX &&
> + (allowed_targets&ATT_INDEX) &&
> + !(allowed_targets&ATT_PARTITIONED_INDEX))
> + /* Add a special errhint for this case, since "is not an index" message is unfriendly */
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg(msg, RelationGetRelationName(rel)),
> + // errhint("\"%s\" is a partitioned index", RelationGetRelationName(rel))));
> + errhint("operation is not supported on partitioned indexes")));
> + else
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg(msg, RelationGetRelationName(rel))));
> +
> }
>
> /*
> --
> 2.7.4
>
--
Justin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, robertmhaas@gmail.com, alvherre@2ndquadrant.com
Subject: Re: ALTER INDEX fails on partitioned index
In-Reply-To: <20200323214704.GM2563@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox