Re: BUG #16368: Incorrect function inlining in the presence of a window function

On Wed, Apr 15, 2020 at 1:08 PM Elvis Pranskevichus <elprans(at)gmail(dot)com>
wrote:

> On Wednesday, April 15, 2020 11:32:54 A.M. PDT David G. Johnston wrote:
> > During inlining the case expression becomes:
> >
> > CASE WHEN q.fmt IS NULL
> > THEN '123,456'::bigint
> > ELSE to_number('123,456', q.fmt)
> > END;
> >
> > It doesn't "take" a branch - it turns variables into constants and, as
> > written, some of those constants are invalid for the types they are
> > being assigned to.
> >
> > > This only happens in the
> > > presence of the "first_value" window call in the nested query.
> >
> > The ability to optimize, and how, depends on the whole query.
> >
> > I don't actually know whether this is a bug or just an expected
> > downside to using inline-able functions and case statements to avoid
> > malformed data parsing.
> >
> > Writing the function in pl/pgsql prevents the inlining and stabilizes
> > the query.
>
> IMO, an optimization that leads to wrong query results is unquestionably
> a bug. And "use pl/pgsql" (which is much slower) is arguably not the
> best answer here.
>

Maybe not, but if you want something that works it is a solution.

> > inline-able functions and case statements to avoid
> > malformed data parsing.
>
> Consider any other case where an error is guarded by a "CASE WHEN", such
> as division by zero.

There aren't all that many and throwing out a perfectly good optimization
for boundary cases that will error, as opposed to returning but including
invalid results, isn't that desirable either.

I think the use of "CASE WHEN" should disqualify
> the function from being inlined, or, maybe, constant folding should be
> disabled in the branches of "CASE WHEN" when inlining and when the
> optimizer is unable to reason about the "CASE" condition.
>
> Thoughts?
>
>
Outside my wheelhouse as to what is practical. I suspect we'll get a
better answer on that front in due time.
The world isn't perfect and I sure cannot write a patch to change this
behavior and for my money its something I could live with if it was decided
that this is the best option at this time.

David J.