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

Jim Jones <jim(dot)jones(at)uni-muenster(dot)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