ALTER TABLE: rework determination of access method ID

Avoid setting an access method OID for relation kinds that don't take
one.  Code review for new feature added in 374c7a229042.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/e5516ac1-5264-c3c0-d822-9e6f614ea93b@gmail.com

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a02c5b05b64432d44e91bc975f5ec269eef976c..6741e721ae352a6c6a6aae49fff2e8eb484daddd 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -958,22 +958,26 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
        }
 
        /*
-        * Select access method to use: an explicitly indicated one, or (in the
-        * case of a partitioned table) the parent's, if it has one.
+        * For relations with table AM and partitioned tables, select access
+        * method to use: an explicitly indicated one, or (in the case of a
+        * partitioned table) the parent's, if it has one.
         */
        if (stmt->accessMethod != NULL)
-               accessMethodId = get_table_am_oid(stmt->accessMethod, false);
-       else if (stmt->partbound)
        {
-               Assert(list_length(inheritOids) == 1);
-               accessMethodId = get_rel_relam(linitial_oid(inheritOids));
+               Assert(RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE);
+               accessMethodId = get_table_am_oid(stmt->accessMethod, false);
        }
-       else
-               accessMethodId = InvalidOid;
+       else if (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE)
+       {
+               if (stmt->partbound)
+               {
+                       Assert(list_length(inheritOids) == 1);
+                       accessMethodId = get_rel_relam(linitial_oid(inheritOids));
+               }
 
-       /* still nothing? use the default */
-       if (RELKIND_HAS_TABLE_AM(relkind) && !OidIsValid(accessMethodId))
-               accessMethodId = get_table_am_oid(default_table_access_method, false);
+               if (RELKIND_HAS_TABLE_AM(relkind) && !OidIsValid(accessMethodId))
+                       accessMethodId = get_table_am_oid(default_table_access_method, false);
+       }
 
        /*
         * Create the relation.  Inherited defaults and constraints are passed in

diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out
index a27805a8f5eb41d061d38006c5ce5d6eb0cccf4d..aa4cb4fa77f9811d0769adba9c9473049d382a65 100644 (file)
--- a/src/test/regress/expected/create_am.out
+++ b/src/test/regress/expected/create_am.out
@@ -547,6 +547,9 @@ CREATE TABLE i_am_a_failure() USING "I do not exist AM";
 ERROR:  access method "I do not exist AM" does not exist
 CREATE TABLE i_am_a_failure() USING "btree";
 ERROR:  access method "btree" is not of type TABLE
+-- Other weird invalid cases that cause problems
+CREATE FOREIGN TABLE fp PARTITION OF pg_am DEFAULT SERVER x;
+ERROR:  "pg_am" is not partitioned
 -- Drop table access method, which fails as objects depends on it
 DROP ACCESS METHOD heap2;
 ERROR:  cannot drop access method heap2 because other objects depend on it

diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql
index adff0079f989728027610dd3a3746efffd6b86d5..c57a87da4b9c38c71357633a4dbd2ddf6220916b 100644 (file)
--- a/src/test/regress/sql/create_am.sql
+++ b/src/test/regress/sql/create_am.sql
@@ -348,6 +348,9 @@ CREATE TABLE i_am_a_failure() USING i_do_not_exist_am;
 CREATE TABLE i_am_a_failure() USING "I do not exist AM";
 CREATE TABLE i_am_a_failure() USING "btree";
 
+-- Other weird invalid cases that cause problems
+CREATE FOREIGN TABLE fp PARTITION OF pg_am DEFAULT SERVER x;
+
 -- Drop table access method, which fails as objects depends on it
 DROP ACCESS METHOD heap2;