Anastasia Lubennikova <a.lubennik...@postgrespro.ru> writes:
> New version is in attachments.
I took a quick look at this and I have a couple of gripes ---
* The naming and documentation of transform_const_function_to_result
seem pretty off-point to me. ISTM the real goal of that function is to
pull up constant values from RTE_FUNCTION RTEs. Replacing the RTE
with an RTE_RESULT is just a side-effect that's needed so that we
don't generate a useless FunctionScan plan node. I'd probably call
it pull_up_constant_function or something like that.
* It would be useful for the commentary to point out that in principle we
could pull up any immutable (or, probably, even just stable) expression;
but we don't, (a) for fear of multiple evaluations of the result costing
us more than we can save, and (b) because a primary goal is to let the
constant participate in further const-folding, and of course that won't
happen for a non-Const.
* The test cases are really pretty bogus, because int4(1) or int4(0) are
not function invocations at all. The parser thinks those are no-op type
casts, and so what comes out of parse analysis is already just a plain
Const. Thus, not one of these test cases is actually verifying that
const-folding of an immutable function has happened before we try to
pull up. While you could dodge the problem today by, say, writing
int8(0) which isn't a no-op cast, I'd recommend staying away from
typename() notation altogether. There's too much baggage there and too
little certainty that you wrote a function call not something else.
The existing test cases you changed, with int4(sin(1)) and so on,
are better examples of something that has to actively be folded to
a constant.
regards, tom lane
