I wrote:
> The whole thing makes me itch a little bit, because this logic is
> assuming that eval_const_expressions is idempotent, something
> we can't test very well and (AFAIR) are not assuming anywhere else.
> If somebody made some sub-case not idempotent, we'd not notice until
> that case got used in a lateral function RTE.
It occurred to me that we could test for problems with double
execution of eval_const_expressions by making planner.c deliberately
call it twice in all cases. A quick hack later, I can say that
"make check-world" doesn't reveal any such problem. While that's
hardly conclusive, it's pretty strong evidence, since the code
coverage report shows eval_const_expressions as well covered (and
the lines not reached are mostly returning simplified Const nodes,
so hardly likely to be a problem here).
We could make a persistent check for that by enclosing the extra
calls in something like "#ifdef DOUBLE_EVAL_CONST_EXPRESSIONS"
and setting up some buildfarm member to define that. But since
(AFAIR) we've never actually had such a bug, it seems unlikely
to be worth the trouble.
I'll go ahead with the patch as presented.
regards, tom lane
