Re: ALTER INDEX fails on partitioned index - Mailing list pgsql-hackers

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