Don't allow partitioned index on foreign-table partitions

Creating indexes on foreign tables is already forbidden, but local
partitioned indexes (commit 8b08f7d4820f) forgot to check for them.  Add
a preliminary check to prevent wasting time.

Another school of thought says to allow the index to be created if it's
not a unique index; but it's possible to do better in the future (enable
indexing of foreign tables, somehow), so we avoid painting ourselves in
a corner by rejecting all cases, to avoid future grief (a.k.a. backward
incompatible changes).

Reported-by: Arseny Sher
Author: Amit Langote, Álvaro Herrera
Discussion: https://postgr.es/m/87sh71cakz.fsf@ars-thinkpad

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 287addf42986361458840392519fb2646141cfd7..bdfb66fa74b365b6912fb16682f944df35739ba3 100644 (file)
--- 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"
 
@@ -1287,7 +1288,6 @@ ProcessUtilitySlow(ParseState *pstate,
                    IndexStmt  *stmt = (IndexStmt *) parsetree;
                    Oid         relid;
                    LOCKMODE    lockmode;
-                   List       *inheritors = NIL;
 
                    if (stmt->concurrent)
                        PreventInTransactionBlock(isTopLevel,
@@ -1314,17 +1314,33 @@ ProcessUtilitySlow(ParseState *pstate,
                     * CREATE INDEX on partitioned tables (but not regular
                     * inherited tables) recurses to partitions, so we must
                     * acquire locks early to avoid deadlocks.
+                    *
+                    * We also take the opportunity to verify that all
+                    * partitions are something we can put an index on,
+                    * to avoid building some indexes only to fail later.
                     */
-                   if (stmt->relation->inh)
+                   if (stmt->relation->inh &&
+                       get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE)
                    {
-                       Relation    rel;
-
-                       /* already locked by RangeVarGetRelidExtended */
-                       rel = heap_open(relid, NoLock);
-                       if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-                           inheritors = find_all_inheritors(relid, lockmode,
-                                                            NULL);
-                       heap_close(rel, NoLock);
+                       ListCell   *lc;
+                       List       *inheritors = NIL;
+
+                       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_INVALID_OBJECT_DEFINITION),
+                                        errmsg("cannot create index on partitioned table \"%s\"",
+                                               stmt->relation->relname),
+                                        errdetail("Table \"%s\" contains partitions that are foreign tables.",
+                                                  stmt->relation->relname)));
+                       }
+                       list_free(inheritors);
                    }
 
                    /* Run parse analysis ... */
@@ -1353,8 +1369,6 @@ ProcessUtilitySlow(ParseState *pstate,
                                                     parsetree);
                    commandCollected = true;
                    EventTriggerAlterTableEnd();
-
-                   list_free(inheritors);
                }
                break;
 

diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 7b54e96d30e26aaacf9f60c62d6db30c12606e88..339a43ff9e47b5c4934c3d20a80d295685dd2506 100644 (file)
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -749,6 +749,13 @@ SELECT * FROM ft1;                                              -- ERROR
 ERROR:  foreign-data wrapper "dummy" has no handler
 EXPLAIN SELECT * FROM ft1;                                      -- ERROR
 ERROR:  foreign-data wrapper "dummy" has no handler
+CREATE TABLE lt1 (a INT) PARTITION BY RANGE (a);
+CREATE FOREIGN TABLE ft_part1
+  PARTITION OF lt1 FOR VALUES FROM (0) TO (1000) SERVER s0;
+CREATE INDEX ON lt1 (a);                                        -- ERROR
+ERROR:  cannot create index on partitioned table "lt1"
+DETAIL:  Table "lt1" contains partitions that are foreign tables.
+DROP TABLE lt1;
 -- ALTER FOREIGN TABLE
 COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
 COMMENT ON FOREIGN TABLE ft1 IS NULL;

diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 63e2e616d71324be86857b39a4f363d2a1124725..c029b2465d958b28803f2e1b1b6b378b9d41844a 100644 (file)
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -316,6 +316,12 @@ CREATE INDEX id_ft1_c2 ON ft1 (c2);                             -- ERROR
 SELECT * FROM ft1;                                              -- ERROR
 EXPLAIN SELECT * FROM ft1;                                      -- ERROR
 
+CREATE TABLE lt1 (a INT) PARTITION BY RANGE (a);
+CREATE FOREIGN TABLE ft_part1
+  PARTITION OF lt1 FOR VALUES FROM (0) TO (1000) SERVER s0;
+CREATE INDEX ON lt1 (a);                                        -- ERROR
+DROP TABLE lt1;
+
 -- ALTER FOREIGN TABLE
 COMMENT ON FOREIGN TABLE ft1 IS 'foreign table';
 COMMENT ON FOREIGN TABLE ft1 IS NULL;