Re: Add notification on BEGIN ATOMIC SQL functions using temp relations

On 23/11/2025 21:20, Tom Lane wrote:
> Pushed with some more editing; for instance there were a bunch
> of comments still oriented toward throwing an error. I also
> still thought the tests were pretty duplicative.

Thanks!

> One note is that I took out
>
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>
> and just let it default to ERRCODE_SUCCESSFUL_COMPLETION,
> which is what happens for the longstanding message about
> temp views too. FEATURE_NOT_SUPPORTED seemed less than
> apropos, and besides this is an error SQLSTATE code not
> a notice/warning code. I don't see any existing SQLSTATE
> that seems on-point here; is it worth inventing one?

Something like ERRCODE_DEPENDENT_TEMP_OBJECT perhaps?

> Before we leave the topic, here's a quick draft of a patch
> to make temp-view detection use the dependency infrastructure
> instead of isQueryUsingTempRelation(). That function is a
> really old, crufty hack that fails to catch all dependencies.
> So this can be sold on the basis of being a functional
> improvement as well as adding detail to the notice messages.

+1

> It's slightly annoying that this patch means we're going to do
> collectDependenciesOfExpr twice on the view body, but the place
> where we'll do that again to update pg_depend is so far removed
> from DefineView() that it seems unduly invasive to try to pass
> down the dependency data. I think it's not that expensive anyway;
> collectDependenciesOfExpr just walks the query tree, I don't believe
> there's any catalog access inside it.
>
> I'm also not super satisfied with the wording of the message for
> the matview case:
>
> if (query_uses_temp_object(query, &temp_object))
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("materialized views must not use temporary objects"),
> errdetail("This view depends on temporary %s.",
> getObjectDescription(&temp_object, false))));
>
> The "It" antecedent doesn't work here because the errmsg doesn't
> state the matview's name, which is also true of a bunch of nearby
> errors. I feel like not a lot of work went into those messages;
> as evidenced by the fact that neither this one nor the other ones
> get tested at all. But I'm not sure I want to do the work to make
> that situation better.

Since there is no test for matviews and temp objects, I guess we could
add this to create_view.sql

-- tests on materialized views depending on temporary objects
CREATE TEMPORARY SEQUENCE temp_seq;
CREATE TEMPORARY VIEW tempmv AS SELECT 1;
CREATE DOMAIN pg_temp.temp_domain AS int CHECK (VALUE > 0);
CREATE TYPE pg_temp.temp_type AS (x int);
-- should fail: temp objects not allowed with MATERIALIZED VIEW
CREATE MATERIALIZED VIEW mv_dep_temptable AS
SELECT * FROM temp_table;
CREATE MATERIALIZED VIEW mv_dep_tempseq AS
SELECT currval('temp_seq');
CREATE MATERIALIZED VIEW mv_dep_tempview AS
SELECT * FROM tempmv;
CREATE MATERIALIZED VIEW mv_dep_tempdomain AS
SELECT 1::pg_temp.temp_domain;
CREATE MATERIALIZED VIEW mv_dep_temptype AS
SELECT (NULL::pg_temp.temp_type).x;

While playing with v9 I realised that this issue also affects
non-temporary tables when they use temporary types (in pg_temp)::

$ psql postgres
psql (19devel)
Type "help" for help.

postgres=# CREATE TYPE pg_temp.temp_type AS (x int);
CREATE TYPE
postgres=# CREATE TABLE tb_temp_type (c pg_temp.temp_type, s serial);
CREATE TABLE
postgres=# INSERT INTO tb_temp_type (c) VALUES
(ROW(42)::pg_temp.temp_type),
(ROW(37)::pg_temp.temp_type);
INSERT 0 2
postgres=# SELECT * FROM tb_temp_type;
c | s
------+---
(42) | 1
(37) | 2
(2 rows)

postgres=# \q

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# SELECT * FROM tb_temp_type;
s
---
1
2
(2 rows)

The column was dropped because it depended on a temporary type. IMO this
is a bit more serious than the previous case, since it silently leads to
data loss. I believe that at least a NOTICE would add some value here.
If so, I can work on a patch for that too.

Best, Jim