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

Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> writes:
> v8 attached.

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.

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?

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.

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.

regards, tom lane