"CREATE RULE ... ON SELECT": redundant?

Started by Ian Lawrence Barwickalmost 3 years ago6 messageshackers
Jump to latest
#1Ian Lawrence Barwick
barwick@gmail.com

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+29-164
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Ian Lawrence Barwick (#1)
Re: "CREATE RULE ... ON SELECT": redundant?

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

#3Ian Lawrence Barwick
barwick@gmail.com
In reply to: Tom Lane (#2)
Re: "CREATE RULE ... ON SELECT": redundant?

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

#4Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Tom Lane (#2)
Re: "CREATE RULE ... ON SELECT": redundant?

"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)

#5Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Andrew Gierth (#4)
Re: "CREATE RULE ... ON SELECT": redundant?

"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)

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Gierth (#5)
Re: "CREATE RULE ... ON SELECT": redundant?

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