Re: Add notification on BEGIN ATOMIC SQL functions using temp relations - Mailing list pgsql-hackers

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