Re: pgsql: SQL-standard function body

On Thu, Apr 8, 2021 at 2:25 AM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> On 08.04.21 01:06, Tom Lane wrote:
> > Andres Freund <andres(at)anarazel(dot)de> writes:
> >> On 2021-04-07 19:53:35 +0000, Peter Eisentraut wrote:
> >>> SQL-standard function body
> >
> >> This is turning the BF red:
> >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2021-04-07%2022%3A52%3A19
> >> Might be force_parallel_mode=regress related.
> >
> > Yeah. On my machine, it's fine without force_parallel_mode and
> > crashes with that. Looks like query text is not getting passed
> > to the parallel worker in some cases.
>
> There does not appear to be any indication in CreateQueryDesc() or for
> es_sourceText that the query source text needs to be not NULL (unlike
> PortalDefineQuery() for example). If we want that to be the case, an
> assert in CreateQueryDesc() would be a straightforward way to catch
> this. But for example code in CreateExecutorState() and
> executor_errposition() is prepared to have es_sourceText be NULL, so
> relative to that, the fix that Andres put into ExecInitParallelPlan() to
> handle that as well seems appropriate.

I think we should insist on having a query string all the time, if at
all possible, because having the query string is awfully nice for
debugging. For example, if a parallel worker dumps core, you'd like to
be able to find out what query it was trying to run when things went
bad. But if we're not going to do that, then I think it would be
better to do something more like what Noah did in
f90e80b9138355a51d2d5b5b63e1f89c4ba53325 rather than what Andres did
here.

--
Robert Haas
EDB: http://www.enterprisedb.com