From: Tom Lane <tgl@sss.pgh.pa.us>
To: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Optimze usage of immutable functions as relation
In-reply-to: <d31c5077-398f-dd26-ef9f-df11918b42f0@postgrespro.ru>
References: <402356c32eeb93d4fed01f66d6c7fe2d@postgrespro.ru>
 <20180906185109.661aaef0@asp437-ThinkPad-L380>
 <153994199621.10161.17815885396545049364.pgcf@coridan.postgresql.org>
 <CACdpekK1oDy7-_HnXOaREa_8HM2r-fsp8iv5e6p8aWOdGdK8Mg@mail.gmail.com>
 <2906.1542395026@sss.pgh.pa.us>
 <3f7fda2f-1cdc-8ad3-e3c9-c2436fb5d509@postgrespro.ru>
 <CA+hUKGK1ATcNa678n=AG+DHQrFmWr-Lovmsq2AP-m5r=shW0Mg@mail.gmail.com>
 <0dd4efb2-22a3-6fef-11f7-1b20dd71a509@postgrespro.ru>
 <d31c5077-398f-dd26-ef9f-df11918b42f0@postgrespro.ru>
Comments: In-reply-to Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
	message dated "Fri, 26 Jul 2019 17:56:05 +0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <18923.1564165560.1@sss.pgh.pa.us>
Date: Fri, 26 Jul 2019 14:26:00 -0400
Message-ID: <18924.1564165560@sss.pgh.pa.us>
Precedence: bulk

Anastasia Lubennikova <a.lubennikova@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