Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

I wrote:
> As to *how* to throw an error, I think it should be possible to teach
> parse analysis to detect such cases, with something like the
> ParseExprKind mechanism that could be checked to see if we're inside
> a subexpression that restricts what's allowed. There are some other
> checks like no-nested-aggregates that perhaps could be folded in as
> well.

I spent some time experimenting with this, and immediately found out
that information_schema.user_mapping_options contains an instance of the
problematic usage :-(. However, that view also turns out to be a poster
child for why our old SRF semantics are a bad idea, because it's on the
hairy edge of failing completely. It's got two SRF invocations in its
tlist, which it relies on to execute in lockstep and produce the same
number of rows --- but then it puts a CASE around one of them, with an
ELSE NULL. So in old versions, that only works because if the CASE
doesn't return a set result at runtime then we just run the tlist the
number of times suggested by the other SRF. And in HEAD, it only works
because we run the two SRFs in lockstep behind the scenes and then the
CASE is discarding individual results not the whole execution of the
second SRF. If you don't have a headache at this point, re-read the above
until you do. Or if you fully understand that and still think it's fine,
consider what will happen if the CASE's test expression generates
different results from one call to the next --- which I think is actually
possible here, because pg_has_role() operates on CatalogSnapshot time and
might possibly change its mind partway through the query. Pre-v10, that
would have generated completely messed-up output, with a hard-to-predict
number of rows and unexpected combinations of the two SRFs' outputs.

Rewriting this view to use a LATERAL SRF call is just enormously cleaner.

Anyway, to the subject at hand. My thought when I wrote the previous
message was to implement a "top down" mechanism whereby contexts like
CASE and COALESCE would record their presence in the ParseState while
recursively analyzing their arguments, and then SRF calls would be
responsible for throwing an error by inspecting that context. The first
attached patch does it that way, and it seems nice and clean, but I ran
into a complete dead end while trying to extend it to handle related cases
such as disallowing SRF-inside-aggregate or nested SRFs in FROM. The
trouble is that when we are parsing the arguments of a SRF or aggregate,
we don't actually know yet whether it's a SRF or aggregate or plain
function. We can't find that out until we look up the pg_proc entry,
which we can't do without knowing the argument types, so we have to do
parse analysis of the arguments first.

I then considered a "bottom up" approach where the idea is for each SRF
to report its presence in the ParseState, and then the outer construct
complains if any SRF reported its presence within the outer construct's
arguments. This isn't hard to implement, just keep a "p_last_srf" pointer
in the ParseState, as in the second attached patch. This feels more ugly
than the first approach; for one thing, if we want to extend it to
multiple cases of "X cannot contain a Y" then we need a ParseState field
for each kind of Y we care about. Also, it will tend to complain about
the last Y within a given X construct, not the first one; but only a true
geek would ever even notice that, let alone feel that it's strange.

I didn't actually fix the "bottom up" approach to complain about nested
SRFs. It's clear to me how to do so, but because we analyze function
arguments before calling ParseFuncOrColumn, we'd have to have the callers
of that function remember the appropriate p_last_srf value (ie, from just
before they analyze the arguments) and then pass it to ParseFuncOrColumn.
That would add a bunch of uninteresting clutter to the patch so I didn't
do it here.

I'm inclined to go with the "bottom up" approach, but I wonder if anyone
has any comments or better ideas?

regards, tom lane