Re: ALTER INDEX fails on partitioned index

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(at)telsasoft(dot)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%40mail.gmail.com
> ---
> 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