Re: Proposal: Progressive explain

Thanks for this valuable testing. I think this is actually a really
good idea for how to test something like this, because the regression
tests contain lots of different queries that do lots of weird things.

On Sun, Mar 30, 2025 at 8:23 PM torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> wrote:
> I haven't looked into the code yet, but when I ran below commands during
> make installcheck, there was an error and an assertion failure
>
> =# select * from pg_stat_progress_explain;
> =# \watch 0.1
>
> ERROR: could not attach to dynamic shared area

This seems like a race condition. Probably some backend's dsa_area
went away between the time we got a pointer to it and the time we
actually attached to it. We should be able to find some way of
handling this without an error, like treating the case where the DSA
area is missing the same as the case where there was no DSA pointer in
the first place. However, this is also making me wonder if we
shouldn't be using one DSA shared by all backends rather than a
separate DSA area for every backend. That would require more care to
avoid leaks, but I'm not sure that it's a good idea to be creating and
destroying a DSA area for every single query. But I'm not 100% sure
that's a problem.

> TRAP: failed Assert("param->paramkind == PARAM_EXTERN"), File:
> "ruleutils.c", Line: 8802, PID: 73180
> TRAP: failed Assert("param->paramkind == PARAM_EXTERN"), File:
> "ruleutils.c", Line: 8802, PID: 73181

I wonder what is happening here. One systemic danger of the patch is
that there can be a difference between what must be true at the *end*
of a query and what must be true *during* a query. Anything that can't
happen at the end but can happen in the middle is something that the
patch will need to do something about in order to work properly. But I
don't see how that can explain this failure, because AFAIR the patch
just prints the same things that would have been printed by any other
EXPLAIN, with the same stack of namespaces. It seems possible that
this is a pre-existing bug: the regression tests might contain a query
that would cause EXPLAIN to fail, but because the regression tests
don't actually EXPLAIN that query, no failure occurs. But it could
also be something else; for example, maybe this patch is trying to
EXPLAIN something that couldn't be used with a regular EXPLAIN for
some reason. Or maybe the patch doesn't actually succeed in doing the
EXPLAIN with the correct namespace stack in all cases.

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