Disallow renaming a rule to "_RETURN".

ON SELECT rules must be named "_RETURN", while other kinds of rules
must not be; this ancient restriction is depended on by various client
code.  We successfully enforced this convention in most places, but
ALTER RULE allowed renaming a non-SELECT rule to "_RETURN".  Notably,
that would break dump/restore, since the eventual CREATE RULE command
would reject the name.

While at it, remove DefineQueryRewrite's hack to substitute "_RETURN"
for the convention that was used before 7.3.  We dropped other
server-side code that supported restoring pre-7.3 dumps some time ago
(notably in e58a59975 and nearby commits), but this bit was missed.

Bug: #19543
Reported-by: Adam Pickering <adamkpickering@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19543-461228e77f3b32fc@postgresql.org
Backpatch-through: 14

diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 6c05a5b989372cf6bb3c0ceaf99b561bed09bdd2..18ba430d9417c9caedd656e8a481dbc3c8ff03ba 100644 (file)
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -390,26 +390,11 @@ DefineQueryRewrite(const char *rulename,
                 * ... and finally the rule must be named _RETURN.
                 */
                if (strcmp(rulename, ViewSelectRuleName) != 0)
-               {
-                       /*
-                        * In versions before 7.3, the expected name was _RETviewname. For
-                        * backwards compatibility with old pg_dump output, accept that
-                        * and silently change it to _RETURN.  Since this is just a quick
-                        * backwards-compatibility hack, limit the number of characters
-                        * checked to a few less than NAMEDATALEN; this saves having to
-                        * worry about where a multibyte character might have gotten
-                        * truncated.
-                        */
-                       if (strncmp(rulename, "_RET", 4) != 0 ||
-                               strncmp(rulename + 4, RelationGetRelationName(event_relation),
-                                               NAMEDATALEN - 4 - 4) != 0)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                                                errmsg("view rule for \"%s\" must be named \"%s\"",
-                                                               RelationGetRelationName(event_relation),
-                                                               ViewSelectRuleName)));
-                       rulename = pstrdup(ViewSelectRuleName);
-               }
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                        errmsg("view rule for \"%s\" must be named \"%s\"",
+                                                       RelationGetRelationName(event_relation),
+                                                       ViewSelectRuleName)));
 
                /*
                 * Are we converting a relation to a view?
@@ -1028,6 +1013,17 @@ RenameRewriteRule(RangeVar *relation, const char *oldName,
                                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                                 errmsg("renaming an ON SELECT rule is not allowed")));
 
+       /*
+        * Conversely, if it's not an ON SELECT rule then it must *not* be named
+        * _RETURN.
+        */
+       if (strcmp(newName, ViewSelectRuleName) == 0)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                errmsg("non-view rule for \"%s\" must not be named \"%s\"",
+                                               RelationGetRelationName(targetrel),
+                                               ViewSelectRuleName)));
+
        /* OK, do the update */
        namestrcpy(&(ruleform->rulename), newName);
 

diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 0a1e17ef5b58b66b7d40b93e10783b92513d059e..bc9fe0f0358700e24e26c09c4820bc051aa17f39 100644 (file)
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3144,6 +3144,8 @@ ALTER RULE NewInsertRule ON rule_v1 RENAME TO "_RETURN"; -- already exists
 ERROR:  rule "_RETURN" for relation "rule_v1" already exists
 ALTER RULE "_RETURN" ON rule_v1 RENAME TO abc; -- ON SELECT rule cannot be renamed
 ERROR:  renaming an ON SELECT rule is not allowed
+ALTER RULE rtest_t4_ins1 ON rtest_t4 RENAME TO "_RETURN"; -- also disallowed
+ERROR:  non-view rule for "rtest_t4" must not be named "_RETURN"
 DROP VIEW rule_v1;
 DROP TABLE rule_t1;
 --

diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index b7ac6407b0dde8381f1e8337a8a6c5047801c605..2a32f8f956fa71f7077bdcc0752f2d829c277084 100644 (file)
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1106,6 +1106,7 @@ SELECT * FROM rule_v1;
 ALTER RULE InsertRule ON rule_v1 RENAME TO NewInsertRule; -- doesn't exist
 ALTER RULE NewInsertRule ON rule_v1 RENAME TO "_RETURN"; -- already exists
 ALTER RULE "_RETURN" ON rule_v1 RENAME TO abc; -- ON SELECT rule cannot be renamed
+ALTER RULE rtest_t4_ins1 ON rtest_t4 RENAME TO "_RETURN"; -- also disallowed
 
 DROP VIEW rule_v1;
 DROP TABLE rule_t1;