"CREATE RULE ... ON SELECT": redundant?
Hi
Commit b23cd185f [1]https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=b23cd185fd5410e5204683933f848d4583e34b35 forbids manual creation of ON SELECT rule on
a table, and updated the main rules documentation [2]https://www.postgresql.org/docs/devel/rules-views.html, but didn't update
the corresponding CREATE RULE page [3]https://www.postgresql.org/docs/devel/sql-createrule.html.
[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
Attachments:
create-rule-on-select-remove-v1.patchtext/x-patch; charset=US-ASCII; name=create-rule-on-select-remove-v1.patchDownload
commit 3cf32a2c68d83e632e37b20be5bad7fb02945750
Author: Ian Barwick <barwick@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
--
Ian Lawrence Barwick <barwick@gmail.com> writes:
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.
What makes you think it's disallowed on views? You do need to use
CREATE OR REPLACE, since the rule will already exist.
regression=# create view v as select * from int8_tbl ;
CREATE VIEW
regression=# create or replace rule "_RETURN" as on select to v do instead select q1, q2+1 as q2 from int8_tbl ;
CREATE RULE
regression=# \d+ v
View "public.v"
Column | Type | Collation | Nullable | Default | Storage | Description
--------+--------+-----------+----------+---------+---------+-------------
q1 | bigint | | | | plain |
q2 | bigint | | | | plain |
View definition:
SELECT int8_tbl.q1,
int8_tbl.q2 + 1 AS q2
FROM int8_tbl;
Now, this is certainly syntax that's deprecated in favor of using
CREATE OR REPLACE VIEW, but I'm very hesitant to remove it. ISTR
that ancient pg_dump files used it in cases involving circular
dependencies. If you want to adjust the docs to say that it's
deprecated in favor of CREATE OR REPLACE VIEW, I could get on
board with that.
regards, tom lane
2023年5月4日(木) 12:51 Tom Lane <tgl@sss.pgh.pa.us>:
Ian Lawrence Barwick <barwick@gmail.com> writes:
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.What makes you think it's disallowed on views? You do need to use
CREATE OR REPLACE, since the rule will already exist.
Ah, "OR REPLACE". Knew I was missing something.
regression=# create view v as select * from int8_tbl ;
CREATE VIEW
regression=# create or replace rule "_RETURN" as on select to v do instead select q1, q2+1 as q2 from int8_tbl ;
CREATE RULE
regression=# \d+ v
View "public.v"
Column | Type | Collation | Nullable | Default | Storage | Description
--------+--------+-----------+----------+---------+---------+-------------
q1 | bigint | | | | plain |
q2 | bigint | | | | plain |
View definition:
SELECT int8_tbl.q1,
int8_tbl.q2 + 1 AS q2
FROM int8_tbl;Now, this is certainly syntax that's deprecated in favor of using
CREATE OR REPLACE VIEW, but I'm very hesitant to remove it. ISTR
that ancient pg_dump files used it in cases involving circular
dependencies. If you want to adjust the docs to say that it's
deprecated in favor of CREATE OR REPLACE VIEW, I could get on
board with that.
'k, I will work on a doc patch.
Thanks
Ian Barwick
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> Now, this is certainly syntax that's deprecated in favor of using
Tom> CREATE OR REPLACE VIEW, but I'm very hesitant to remove it. ISTR
Tom> that ancient pg_dump files used it in cases involving circular
Tom> dependencies.
I thought they used CREATE RULE on a table?
In fact here is an example from a pg 9.5 pg_dump output (with cruft
removed):
CREATE TABLE public.cdep (
a integer,
b text
);
CREATE FUNCTION public.cdep_impl() RETURNS SETOF public.cdep
LANGUAGE plpgsql
AS $$ begin return query select a,b from (values (1,'foo'),(2,'bar')) v(a,b); end; $$;
CREATE RULE "_RETURN" AS
ON SELECT TO public.cdep DO INSTEAD SELECT cdep_impl.a,
cdep_impl.b
FROM public.cdep_impl() cdep_impl(a, b);
and this now fails to restore:
psql:t1.sql:68: ERROR: relation "cdep" cannot have ON SELECT rules
DETAIL: This operation is not supported for tables.
--
Andrew (irc:RhodiumToad)
"Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
Andrew> I thought they used CREATE RULE on a table?
Andrew> In fact here is an example from a pg 9.5 pg_dump output (with
Andrew> cruft removed):
And checking other versions, 9.6 is the same, it's only with pg 10 that
it switches to creating a dummy view instead of a table (and using
CREATE OR REPLACE VIEW, no mention of rules).
So if the goal was to preserve compatibility with pre-pg10 dumps, that's
already broken; if that's ok, then I don't see any obvious reason not to
also remove or at least deprecate CREATE RULE ... ON SELECT for views.
--
Andrew (irc:RhodiumToad)
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
And checking other versions, 9.6 is the same, it's only with pg 10 that
it switches to creating a dummy view instead of a table (and using
CREATE OR REPLACE VIEW, no mention of rules).
9.6.16 or later will use CREATE OR REPLACE VIEW, cf 404cbc562.
So if the goal was to preserve compatibility with pre-pg10 dumps, that's
already broken; if that's ok, then I don't see any obvious reason not to
also remove or at least deprecate CREATE RULE ... ON SELECT for views.
Since the CREATE OR REPLACE case still works, I don't think removing
it is OK.
regards, tom lane