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 <pryz...@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
