Thread: EphemeralNamedRelation and materialized view
Hi,
While looking into the commit b4da732fd64e936970f38c792f8b32c4bdf2bcd5,
I noticed that we can create a materialized view using Ephemeral Named
Relation in PostgreSQL 16 or earler.
postgres=# create table tbl (i int);
CREATE TABLE
^
postgres=# create or replace function f() returns trigger as $$ begin
create materialized view mv as select * from enr; return new; end; $$ language plpgsql;
CREATE FUNCTION
postgres=# create trigger trig after insert on tbl referencing new table as enr execute function f();
CREATE TRIGGER
postgres=# insert into tbl values (10);
postgres=# \d
List of relations
Schema | Name | Type | Owner
--------+------+-------------------+--------
public | mv | materialized view | yugo-n
public | tbl | table | yugo-n
(2 rows)
We cannot refresh or get the deinition of it, though.
postgres=# refresh materialized view mv;
ERROR: executor could not find named tuplestore "enr"
postgres=# \d+ mv
ERROR: unrecognized RTE kind: 7
In PostgreSQL 17, materialized view using ENR cannot be created
because queryEnv is not pass to RefreshMatViewByOid introduced by b4da732fd64.
When we try to create it, the error is raised.
ERROR: executor could not find named tuplestore "enr"
Although it is hard to imagine users actually try to create materialized view
using ENR, how about prohibiting it even in PG16 or earlier by passing NULL
as queryEnv arg in CreateQueryDesc to avoid to create useless matviews accidentally,
as the attached patch?
Regards,
Yugo Nagata
--
Yugo Nagata <nagata@sraoss.co.jp>
Attachment
On Fri, 26 Jul 2024 at 12:07, Yugo Nagata <nagata@sraoss.co.jp> wrote: > > Hi, > > While looking into the commit b4da732fd64e936970f38c792f8b32c4bdf2bcd5, > I noticed that we can create a materialized view using Ephemeral Named > Relation in PostgreSQL 16 or earler. > > > postgres=# create table tbl (i int); > CREATE TABLE > ^ > postgres=# create or replace function f() returns trigger as $$ begin > create materialized view mv as select * from enr; return new; end; $$ language plpgsql; > CREATE FUNCTION > > postgres=# create trigger trig after insert on tbl referencing new table as enr execute function f(); > CREATE TRIGGER > > postgres=# insert into tbl values (10); > > postgres=# \d > List of relations > Schema | Name | Type | Owner > --------+------+-------------------+-------- > public | mv | materialized view | yugo-n > public | tbl | table | yugo-n > (2 rows) > > > We cannot refresh or get the deinition of it, though. > > postgres=# refresh materialized view mv; > ERROR: executor could not find named tuplestore "enr" > > postgres=# \d+ mv > ERROR: unrecognized RTE kind: 7 > > In PostgreSQL 17, materialized view using ENR cannot be created > because queryEnv is not pass to RefreshMatViewByOid introduced by b4da732fd64. > When we try to create it, the error is raised. > > ERROR: executor could not find named tuplestore "enr" > > Although it is hard to imagine users actually try to create materialized view > using ENR, how about prohibiting it even in PG16 or earlier by passing NULL > as queryEnv arg in CreateQueryDesc to avoid to create useless matviews accidentally, > as the attached patch? > > > Regards, > Yugo Nagata > > -- > Yugo Nagata <nagata@sraoss.co.jp> Hi I think this is a clear bug fix, and should be backported in pg v12-v16. LTGM P.S should be set https://commitfest.postgresql.org/49/5153/ entry as RFC? -- Best regards, Kirill Reshke
Yugo Nagata <nagata@sraoss.co.jp> writes:
> While looking into the commit b4da732fd64e936970f38c792f8b32c4bdf2bcd5,
> I noticed that we can create a materialized view using Ephemeral Named
> Relation in PostgreSQL 16 or earler.
Yeah, we should reject that, but I feel like this patch is not
ambitious enough, because the 17-and-up behavior isn't exactly
polished either.
I tried variants of this function in HEAD:
1. With "create table mv as select * from enr", it works and
does what you'd expect.
2. With "create view mv as select * from enr", you get
regression=# insert into tbl values (10);
ERROR: relation "enr" does not exist
LINE 1: create view mv as select * from enr
^
QUERY: create view mv as select * from enr
CONTEXT: PL/pgSQL function f() line 2 at SQL statement
regression=# \errverbose
ERROR: 42P01: relation "enr" does not exist
LINE 1: create view mv as select * from enr
^
QUERY: create view mv as select * from enr
CONTEXT: PL/pgSQL function f() line 2 at SQL statement
LOCATION: parserOpenTable, parse_relation.c:1452
3. With "create materialized view ..." you get
regression=# insert into tbl values (10);
ERROR: executor could not find named tuplestore "enr"
CONTEXT: SQL statement "create materialized view mv as select * from enr"
PL/pgSQL function f() line 2 at SQL statement
regression=# \errverbose
ERROR: XX000: executor could not find named tuplestore "enr"
CONTEXT: SQL statement "create materialized view mv as select * from enr"
PL/pgSQL function f() line 2 at SQL statement
LOCATION: ExecInitNamedTuplestoreScan, nodeNamedtuplestorescan.c:107
I don't think hitting an internal error is good enough.
Why doesn't this case act like case 2?
You could even argue that case 2 isn't good enough either,
and we should be delivering a specific error message saying
that an ENR can't be used in a view/matview. To do that,
we'd likely need to pass down the QueryEnvironment in more
places not fewer.
regards, tom lane
Yugo NAGATA <nagata@sraoss.co.jp> writes:
>> You could even argue that case 2 isn't good enough either,
>> and we should be delivering a specific error message saying
>> that an ENR can't be used in a view/matview. To do that,
>> we'd likely need to pass down the QueryEnvironment in more
>> places not fewer.
> We can raise a similar error for (not materialized) views by passing
> QueryEnv to DefineView() (or in ealier stage) , but there are other
> objects that can contain ENR in their definition, for examle, functions,
> cursor, or RLS policies. Is it worth introducing this version of error
> message for all these objects?
If it's worth checking for here, why not in other cases?
I'm not sure I like using isQueryUsingTempRelation as a model,
because its existing use in transformCreateTableAsStmt seems
like mostly a hack. (And I definitely don't love introducing
yet another scan of the query.) It seems to me that we should
think about this, for MVs as well as those other object types,
as fundamentally a dependency problem. That is, the reason
we can't allow a reference to an ENR in a long-lived object
is that we have no catalog representation for the reference.
So that leads to thinking that the issue ought to be handled
in recordDependencyOnExpr and friends. If we see an ENR while
scanning a rangetable to extract dependencies, then complain.
This might be a bit messy to produce good error messages for,
though.
Speaking of error messages, I'm not sure that it's okay to
use the phrase "ephemeral named relation" in a user-facing
error message. We don't use that term in our documentation
AFAICS, except in some SPI documentation that most users
will never have read. In the context of triggers, "transition
relation" seems to be what the docs use.
regards, tom lane
On Fri, 15 Nov 2024 at 13:37, Yugo NAGATA <nagata@sraoss.co.jp> wrote: > > On Sun, 03 Nov 2024 13:42:33 -0500 > Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > Yugo Nagata <nagata@sraoss.co.jp> writes: > > > While looking into the commit b4da732fd64e936970f38c792f8b32c4bdf2bcd5, > > > I noticed that we can create a materialized view using Ephemeral Named > > > Relation in PostgreSQL 16 or earler. > > > > Yeah, we should reject that, but I feel like this patch is not > > ambitious enough, because the 17-and-up behavior isn't exactly > > polished either. > > > > I tried variants of this function in HEAD: > > > > 1. With "create table mv as select * from enr", it works and > > does what you'd expect. > > > > 2. With "create view mv as select * from enr", you get > > > > regression=# insert into tbl values (10); > > ERROR: relation "enr" does not exist > > LINE 1: create view mv as select * from enr > > ^ > > QUERY: create view mv as select * from enr > > CONTEXT: PL/pgSQL function f() line 2 at SQL statement > > regression=# \errverbose > > ERROR: 42P01: relation "enr" does not exist > > LINE 1: create view mv as select * from enr > > ^ > > QUERY: create view mv as select * from enr > > CONTEXT: PL/pgSQL function f() line 2 at SQL statement > > LOCATION: parserOpenTable, parse_relation.c:1452 > > > > 3. With "create materialized view ..." you get > > > > regression=# insert into tbl values (10); > > ERROR: executor could not find named tuplestore "enr" > > CONTEXT: SQL statement "create materialized view mv as select * from enr" > > PL/pgSQL function f() line 2 at SQL statement > > regression=# \errverbose > > ERROR: XX000: executor could not find named tuplestore "enr" > > CONTEXT: SQL statement "create materialized view mv as select * from enr" > > PL/pgSQL function f() line 2 at SQL statement > > LOCATION: ExecInitNamedTuplestoreScan, nodeNamedtuplestorescan.c:107 > > > > I don't think hitting an internal error is good enough. > > Why doesn't this case act like case 2? > > I agree that raising an internal error is not enough. I attached a updated > patch that outputs a message saying that an ENR can't be used in a matview. > > > You could even argue that case 2 isn't good enough either, > > and we should be delivering a specific error message saying > > that an ENR can't be used in a view/matview. To do that, > > we'd likely need to pass down the QueryEnvironment in more > > places not fewer. > > We can raise a similar error for (not materialized) views by passing > QueryEnv to DefineView() (or in ealier stage) , but there are other > objects that can contain ENR in their definition, for examle, functions, > cursor, or RLS policies. Is it worth introducing this version of error > message for all these objects? > > Regards, > Yugo Nagata > > -- > Yugo NAGATA <nagata@sraoss.co.jp> Hi! There are review comments that need to be addressed. Commitfest status is now waiting on the author. [0] https://www.postgresql.org/message-id/ZzrHUEaWB67EAZpW%40paquier.xyz [1] https://www.postgresql.org/message-id/222722.1732124596%40sss.pgh.pa.us -- Best regards, Kirill Reshke