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

From Tom Lane
Subject Re: Add notification on BEGIN ATOMIC SQL functions using temp relations
Date
Msg-id 670381.1762288885@sss.pgh.pa.us
Whole thread Raw
In response to Re: Add notification on BEGIN ATOMIC SQL functions using temp relations  (Jim Jones <jim.jones@uni-muenster.de>)
Responses Re: Add notification on BEGIN ATOMIC SQL functions using temp relations
List pgsql-hackers
Jim Jones <jim.jones@uni-muenster.de> writes:
> Oops... wrong files. Sorry.
> PFA the correct version.

A few thoughts:

0001 is mostly what I had in mind, except that I do not think
collectDependenciesFromExpr should perform
eliminate_duplicate_dependencies; it should be explicitly documented
that the caller should do that before recording the dependencies.
This approach will avoid duplicate work when collecting dependencies
from multiple sources.

It seems like a lot of the changes in recordDependencyOnSingleRelExpr,
maybe all of them, were unnecessary --- why'd you find it useful to
add the "addrs" variable instead of continuing to use context.addrs?

Nitpick: it looks like the comments in 0001 are mostly written to
fit into a window that's a bit wider than 80 characters.  Our standard
is to keep lines to 80 or less characters.

I'm not terribly happy with 0002.  In particular, I don't like
filter_temp_objects having an explicit list of which object types
might be temp.  But I don't think we need to do that, because
the objectaddress.c infrastructure already knows all about
which objects belong to schemas.  I think you can just use
get_object_namespace(), and if it returns something that satisfies
OidIsValid(namespace) && isAnyTempNamespace(namespace),
then complain.  (Also, use getObjectDescription() to build a
description of what you're complaining about, rather than
hard-coding that knowledge.)

The bigger issue is that it's not only the prosqlbody that
we ought to be applying this check to.  For example, we should
similarly refuse cases where a temporary type is used as an
argument or result type.  So I think the way that ProcedureCreate
needs to work is to collect up *all* of the dependencies that
it is creating into an ObjectAddresses list, and then de-dup
that (once), check it for temp references, and finally record it.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: meson's in-tree libpq header search order vs -Dextra_include_dirs
Next
From: "Joel Jacobson"
Date:
Subject: Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue