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

Jim Jones <jim.jones@uni-muenster.de> writes:
> [ v3-0001-Disallow-ATOMIC-functions-depending-on-temp-relat.patch ]

Got around to reading the patch finally.  I don't like anything
about this implementation.  It introduces yet another place that
(thinks it) knows how to find all the dependencies in a query
tree, requiring yet another scan of the function's tree, and yet
it is quite incomplete.

Also, I don't think fmgr_sql_validator is a great place to drive
this from, especially not where you put the work, because that
doesn't run if check_function_bodies is turned off.

I think the right way to make this work is to look through the
array of ObjectAddresses that ProcedureCreate builds to store
into pg_depend, because that is by definition the authoritative
info about what the function is dependent on.  There's some
refactoring pain to be endured to make that happen though.
Most of the interesting-for-this-purpose dependencies are
found by recordDependencyOnExpr, which summarily writes them
out before we'd get a chance to look at them.  I think what we
want to do is refactor that so that we have a function along
the lines of "add all the dependencies of this expression to
a caller-supplied ObjectAddresses struct".  Then merge the
dependencies found by that function into the list of special
dependencies that ProcedureCreate has hard-wired logic for, then
de-duplicate that list, then (if not a temp function) scan the
list for dependencies on temp objects, and finally (if no error)
write it out to pg_depend using recordMultipleDependencies.
This would provide more effective de-duplication of pg_depend
entries than what ProcedureCreate is doing today, and it would
give us full coverage not just partial.

I realize that you probably cribbed this logic from
isQueryUsingTempRelation, but that is looking pretty sad too.
As a concrete example of what I'm talking about:

regression=# create temp table mytemp (f1 int);
CREATE TABLE
regression=# create view vfoo as select * from pg_class where oid = 'mytemp'::regclass;
CREATE VIEW
regression=# \c -
You are now connected to database "regression" as user "postgres".
regression=# \d vfoo
Did not find any relation named "vfoo".

because recordDependencyOnExpr knows that a regclass constant
creates a dependency, but isQueryUsingTempRelation doesn't.
So we might want to up our game for detecting views that should
be temp in a similar fashion, ie merge the test with collection
of the object's real dependencies.

            regards, tom lane