Hi

Commit b23cd185f [1] forbids manual creation of ON SELECT rule on
a table, and updated the main rules documentation [2], but didn't update
the corresponding CREATE RULE page [3].

[1] 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=b23cd185fd5410e5204683933f848d4583e34b35
[2] https://www.postgresql.org/docs/devel/rules-views.html
[3] https://www.postgresql.org/docs/devel/sql-createrule.html

While poking around at an update for that, unless I'm missing something it is
now not possible to use "CREATE RULE ... ON SELECT" for any kind of relation,
given that it's disallowed on views / material views already.

Assuming that's the case, that makes this useless syntax in a non-SQL-standard
command, is there any reason to keep it in the grammar at all?

Attached suggested patch removes it entirely and updates the CREATE RULE
documentation.

Apart from removing ON SELECT from the grammar, the main change is the removal
of usage checks in DefineQueryRewrite(), as the only time it is called with the
event_type set to "CMD_SELECT" is when a view/matview is created, and presumably
we can trust the internal caller to do the right thing. I added an Assert in
just in case, dunno if that's really needed. In passing, a redundant workaround
for pre-7.3 rule names gets removed as well.

I note that with or without this change, pg_get_ruledef() e.g. executed with:

  SELECT pg_get_ruledef(oid) FROM pg_rewrite WHERE
ev_class='some_view'::regclass;

emits SQL for CREATE RULE which can no longer be executed; I don't think there
is anything which can be done about that other than noting it as a historical
implementation oddity.


Regards

Ian Barwick
commit 3cf32a2c68d83e632e37b20be5bad7fb02945750
Author: Ian Barwick <barw...@gmail.com>
Date:   Tue May 2 22:17:47 2023 +0900

    Remove SQL syntax support for CREATE RULE ... ON SELECT.
    
    Commit b23cd185f forbids creation of an ON SELECT rule on tables,
    which means it is no longer possible to create an ON SELECT rule
    on any relation type, making the ON SELECT event option entirely
    redundant.
    
    This patch removes the syntax, and also updates the CREATE RULE
    documentation page.

diff --git a/doc/src/sgml/ref/create_rule.sgml b/doc/src/sgml/ref/create_rule.sgml
index dbf4c93784..0f031bdc2f 100644
--- a/doc/src/sgml/ref/create_rule.sgml
+++ b/doc/src/sgml/ref/create_rule.sgml
@@ -27,7 +27,7 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
 
 <phrase>where <replaceable class="parameter">event</replaceable> can be one of:</phrase>
 
-    SELECT | INSERT | UPDATE | DELETE
+    INSERT | UPDATE | DELETE
 </synopsis>
  </refsynopsisdiv>
 
@@ -58,18 +58,6 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
    More information about the rules system is in <xref linkend="rules"/>.
   </para>
 
-  <para>
-   Presently, <literal>ON SELECT</literal> rules must be unconditional
-   <literal>INSTEAD</literal> rules and must have actions that consist
-   of a single <command>SELECT</command> command.  Thus, an
-   <literal>ON SELECT</literal> rule effectively turns the table into
-   a view, whose visible contents are the rows returned by the rule's
-   <command>SELECT</command> command rather than whatever had been
-   stored in the table (if anything).  It is considered better style
-   to write a <command>CREATE VIEW</command> command than to create a
-   real table and define an <literal>ON SELECT</literal> rule for it.
-  </para>
-
   <para>
    You can create the illusion of an updatable view by defining
    <literal>ON INSERT</literal>, <literal>ON UPDATE</literal>, and
@@ -134,12 +122,11 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
     <term><replaceable class="parameter">event</replaceable></term>
     <listitem>
      <para>
-      The event is one of <literal>SELECT</literal>,
-      <literal>INSERT</literal>, <literal>UPDATE</literal>, or
-      <literal>DELETE</literal>.  Note that an
-      <command>INSERT</command> containing an <literal>ON
-      CONFLICT</literal> clause cannot be used on tables that have
-      either <literal>INSERT</literal> or <literal>UPDATE</literal>
+      The event is one of <literal>INSERT</literal>,
+      <literal>UPDATE</literal>, or <literal>DELETE</literal>.
+      Note that an <command>INSERT</command> containing an
+      <literal>ON CONFLICT</literal> clause cannot be used on tables
+      that have either <literal>INSERT</literal> or <literal>UPDATE</literal>
       rules.  Consider using an updatable view instead.
      </para>
     </listitem>
@@ -246,22 +233,22 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
    It is very important to take care to avoid circular rules.  For
    example, though each of the following two rule definitions are
    accepted by <productname>PostgreSQL</productname>, the
-   <command>SELECT</command> command would cause
+   <command>INSERT</command> command would cause
    <productname>PostgreSQL</productname> to report an error because
    of recursive expansion of a rule:
 
 <programlisting>
-CREATE RULE "_RETURN" AS
-    ON SELECT TO t1
-    DO INSTEAD
-        SELECT * FROM t2;
+CREATE RULE t1_insert AS
+  ON INSERT TO t1
+  DO INSTEAD
+    INSERT INTO t2 VALUES (NEW.id);
 
-CREATE RULE "_RETURN" AS
-    ON SELECT TO t2
-    DO INSTEAD
-        SELECT * FROM t1;
+CREATE RULE t2_insert AS
+  ON INSERT TO t2
+  DO INSTEAD
+    INSERT INTO t1 VALUES (NEW.id);
 
-SELECT * FROM t1;
+INSERT INTO t1 VALUES(1);
 </programlisting>
   </para>
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a723d9db78..cf74f75792 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10781,8 +10781,7 @@ RuleActionStmtOrEmpty:
 			|	/*EMPTY*/							{ $$ = NULL; }
 		;
 
-event:		SELECT									{ $$ = CMD_SELECT; }
-			| UPDATE								{ $$ = CMD_UPDATE; }
+event:		UPDATE									{ $$ = CMD_UPDATE; }
 			| DELETE_P								{ $$ = CMD_DELETE; }
 			| INSERT								{ $$ = CMD_INSERT; }
 		 ;
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index e36fc72e1e..b0c34c1c9c 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -305,118 +305,15 @@ DefineQueryRewrite(const char *rulename,
 					 errhint("Use triggers instead.")));
 	}
 
+	/*
+	 * From PostgreSQL 16, ON SELECT rules can only be created internally
+	 * when executing CREATE VIEW / CREATE MATERIALIZED VIEW, so there's
+	 * no need to check for correct usage here.
+	 */
 	if (event_type == CMD_SELECT)
 	{
-		/*
-		 * Rules ON SELECT are restricted to view definitions
-		 *
-		 * So this had better be a view, ...
-		 */
-		if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
-			event_relation->rd_rel->relkind != RELKIND_MATVIEW)
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("relation \"%s\" cannot have ON SELECT rules",
-							RelationGetRelationName(event_relation)),
-					 errdetail_relkind_not_supported(event_relation->rd_rel->relkind)));
-
-		/*
-		 * ... there cannot be INSTEAD NOTHING, ...
-		 */
-		if (action == NIL)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("INSTEAD NOTHING rules on SELECT are not implemented"),
-					 errhint("Use views instead.")));
-
-		/*
-		 * ... there cannot be multiple actions, ...
-		 */
-		if (list_length(action) > 1)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("multiple actions for rules on SELECT are not implemented")));
-
-		/*
-		 * ... the one action must be a SELECT, ...
-		 */
-		query = linitial_node(Query, action);
-		if (!is_instead ||
-			query->commandType != CMD_SELECT)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("rules on SELECT must have action INSTEAD SELECT")));
-
-		/*
-		 * ... it cannot contain data-modifying WITH ...
-		 */
-		if (query->hasModifyingCTE)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("rules on SELECT must not contain data-modifying statements in WITH")));
-
-		/*
-		 * ... there can be no rule qual, ...
-		 */
-		if (event_qual != NULL)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("event qualifications are not implemented for rules on SELECT")));
-
-		/*
-		 * ... the targetlist of the SELECT action must exactly match the
-		 * event relation, ...
-		 */
-		checkRuleResultList(query->targetList,
-							RelationGetDescr(event_relation),
-							true,
-							event_relation->rd_rel->relkind !=
-							RELKIND_MATVIEW);
-
-		/*
-		 * ... there must not be another ON SELECT rule already ...
-		 */
-		if (!replace && event_relation->rd_rules != NULL)
-		{
-			int			i;
-
-			for (i = 0; i < event_relation->rd_rules->numLocks; i++)
-			{
-				RewriteRule *rule;
-
-				rule = event_relation->rd_rules->rules[i];
-				if (rule->event == CMD_SELECT)
-					ereport(ERROR,
-							(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-							 errmsg("\"%s\" is already a view",
-									RelationGetRelationName(event_relation))));
-			}
-		}
-
-		/*
-		 * ... 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);
-		}
+		Assert(event_relation->rd_rel->relkind == RELKIND_VIEW
+			   || event_relation->rd_rel->relkind == RELKIND_MATVIEW);
 	}
 	else
 	{
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 69957687fe..1256e554fb 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2735,25 +2735,14 @@ HINT:  You can drop view rules_fooview instead.
 drop view rules_fooview;
 --
 -- We used to allow converting a table to a view by creating a "_RETURN"
--- rule for it, but no more.
+-- rule for it, but the "ON SELECT" syntax has been removed.
 --
 create table rules_fooview (x int, y text);
 create rule "_RETURN" as on select to rules_fooview do instead
   select 1 as x, 'aaa'::text as y;
-ERROR:  relation "rules_fooview" cannot have ON SELECT rules
-DETAIL:  This operation is not supported for tables.
-drop table rules_fooview;
--- likewise, converting a partitioned table or partition to view is not allowed
-create table rules_fooview (x int, y text) partition by list (x);
-create rule "_RETURN" as on select to rules_fooview do instead
-  select 1 as x, 'aaa'::text as y;
-ERROR:  relation "rules_fooview" cannot have ON SELECT rules
-DETAIL:  This operation is not supported for partitioned tables.
-create table rules_fooview_part partition of rules_fooview for values in (1);
-create rule "_RETURN" as on select to rules_fooview_part do instead
-  select 1 as x, 'aaa'::text as y;
-ERROR:  relation "rules_fooview_part" cannot have ON SELECT rules
-DETAIL:  This operation is not supported for tables.
+ERROR:  syntax error at or near "select"
+LINE 1: create rule "_RETURN" as on select to rules_fooview do inste...
+                                    ^
 drop table rules_fooview;
 --
 -- check for planner problems with complex inherited UPDATES
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 4caab3434b..d07e917894 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -885,7 +885,7 @@ drop view rules_fooview;
 
 --
 -- We used to allow converting a table to a view by creating a "_RETURN"
--- rule for it, but no more.
+-- rule for it, but the "ON SELECT" syntax has been removed.
 --
 
 create table rules_fooview (x int, y text);
@@ -893,17 +893,6 @@ create rule "_RETURN" as on select to rules_fooview do instead
   select 1 as x, 'aaa'::text as y;
 drop table rules_fooview;
 
--- likewise, converting a partitioned table or partition to view is not allowed
-create table rules_fooview (x int, y text) partition by list (x);
-create rule "_RETURN" as on select to rules_fooview do instead
-  select 1 as x, 'aaa'::text as y;
-
-create table rules_fooview_part partition of rules_fooview for values in (1);
-create rule "_RETURN" as on select to rules_fooview_part do instead
-  select 1 as x, 'aaa'::text as y;
-
-drop table rules_fooview;
-
 --
 -- check for planner problems with complex inherited UPDATES
 --

Reply via email to