Alexander Pyhalov <a.pyhalov@postgrespro.ru> writes:
> [ updated patch ]
Thanks for updating the patch. (BTW, please attach version numbers
to new patch versions, to avoid confusion.)
However, before we proceed any further with this patch, I think we
really ought to stop and think about the question I raised last
night: why are we building a one-off feature for SQLValueFunction?
Wouldn't the same parameter-substitution mechanism work for *any*
stable expression that doesn't contain remote Vars? That would
subsume the now() case as well as plenty of others.
So far the only counterexample I've been able to come up with is
that shipping values of reg* types might not be too safe, because
the remote side might not have the same objects. For example
consider these two potential quals:
WHERE remote_oid_column = CURRENT_ROLE::regrole
WHERE remote_text_column = CURRENT_ROLE::text
Say we're running as user 'joe' and that role doesn't exist on the
remote server. Then executing the first WHERE locally is fine, but
shipping it to the remote would cause a failure because the remote's
regrolein() will fail to convert the parameter value. But the second
case is quite non-problematic, because what will be sent over is just
some uninterpreted text.
In point of fact, this hazard doesn't have anything to do with stable
or not-stable subexpressions --- for example,
WHERE remote_oid_column = 'joe'::regrole
is just as unsafe, even though the value under consideration is a
*constant*. Maybe there is something in postgres_fdw that would stop
it from shipping this qual, but I don't recall seeing it, so I wonder
if there's a pre-existing bug here.
So it seems like we need a check to prevent generating remote Params
that are of "unsafe" types, but this is a type issue not an expression
issue --- as long as an expression is stable and does not yield an
unsafe-to-ship data type, why can't we treat it as a Param?
regards, tom lane