On 2018/05/10 10:02, Michael Paquier wrote: > Something > that I find confusing on HEAD though is that DefineIndex calls itself > around line 1006 and cascades through each children but there is no > context about the error. > > For example if I have this partition layer: > CREATE TABLE measurement ( > logdate date not null, > peaktemp int, > unitsales int > ) PARTITION BY RANGE (logdate); > CREATE FOREIGN TABLE measurement_y2016m07 > PARTITION OF measurement FOR VALUES FROM ('2016-07-01') TO > ('2016-08-01') > SERVER postgres_server; > > Then by creating an index on the parent I get that: > =# create index measurement_idx on measurement (peaktemp); > ERROR: 42809: cannot create index on foreign table "measurement_y2016m07" > LOCATION: DefineIndex, indexcmds.c:442 > > This may be confusing for the user as the DDL does not complain directly > about the relation worked on. Perhaps we could add an error context? > We surely don't want multiple contexts either when working on long > chains, but we could build a chained list of relation names in the error > message.
How about we error out even *before* calling DefineIndex for the 1st time? I see that ProcessUtilitySlow() gets a list of all partitions when locking them for index creation before calling DefineIndex. Maybe, just go through the list and error out if one of them is a partition that we don't support creating an index on? For example, with the attached patch doing the above, I get: create table p (a int) partition by range (a); create table p1 partition of p for values from (minvalue) to (1); create foreign table p2 partition of p for values from (1) to (maxvalue) server loopback options (table_name 'p2base'); create index on p (a); ERROR: cannot create index on partitioned table containing foreign partitions Thanks, Amit
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 287addf429..d49e32d83f 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -67,6 +67,7 @@ #include "tcop/utility.h" #include "utils/acl.h" #include "utils/guc.h" +#include "utils/lsyscache.h" #include "utils/syscache.h" #include "utils/rel.h" @@ -1318,12 +1319,26 @@ ProcessUtilitySlow(ParseState *pstate, if (stmt->relation->inh) { Relation rel; + ListCell *lc; /* already locked by RangeVarGetRelidExtended */ rel = heap_open(relid, NoLock); if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) inheritors = find_all_inheritors(relid, lockmode, NULL); + foreach(lc, inheritors) + { + char relkind = get_rel_relkind(lfirst_oid(lc)); + + if (relkind != RELKIND_RELATION && + relkind != RELKIND_MATVIEW && + relkind != RELKIND_PARTITIONED_TABLE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot create index on partitioned table containing foreign partitions"))); + } + + list_free(inheritors); heap_close(rel, NoLock); } @@ -1353,8 +1368,6 @@ ProcessUtilitySlow(ParseState *pstate, parsetree); commandCollected = true; EventTriggerAlterTableEnd(); - - list_free(inheritors); } break;