public inbox for [email protected]
help / color / mirror / Atom feedFrom: Robert Haas <[email protected]>
To: torikoshia <[email protected]>
Cc: Rafael Thofehrn Castro <[email protected]>
Cc: Andrei Lepikhov <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Proposal: Progressive explain
Date: Mon, 31 Mar 2025 09:30:34 -0400
Message-ID: <CA+TgmobUpRO4j-cYzOHnDMuksZiRy87nPER2bkFamtpthmxoaw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAG0ozMpD27fCyVFoa82G7uV4md4wVYw7DWC-PVA6P5=1v35tsA@mail.gmail.com>
<CAG0ozMqfQjO=xK-zwfvNGvpZnWtynB6iXxCShcH5t5vsYV7_LA@mail.gmail.com>
<CAG0ozMoaV_C1_Sf7QMjfCjUODL8U959gvYWPrVSjgvc2gcRyxQ@mail.gmail.com>
<CAG0ozMo30smtXXOR8bSCbhaZAQHo4=ezerLitpERk85Q0ga+Fw@mail.gmail.com>
<CAG0ozMo4eaJJqqiHP7dHXm=u8vK-qqxo5Q1DVu=amHisZiX5_g@mail.gmail.com>
<[email protected]>
<CAG0ozMpwRQFNraWLGVa0zAFBYuY1HasZ6NX4BAM4H3OOrQHtDg@mail.gmail.com>
<CAG0ozMrZ+dG9TA+X54Tjwxm76O2_u97_Yvhdf3ixx+0caGF4Mg@mail.gmail.com>
<CA+TgmoaD985+VLwR93c8PjSaoBqxw72Eu7pfBJcArzhjJ71aRw@mail.gmail.com>
<CAG0ozMqcsZFmaeodUA9tYbbZsYJFYgJVYxTpa=ggee+ShvWQCA@mail.gmail.com>
<CA+TgmoZw+xy3WyrPD4X5ChP+=kJ4ncRZY8xQu0XfChDa4fa7yg@mail.gmail.com>
<CAG0ozMpjEFThDGNqSoFTj1v_cpEs0rRQw5moVKU6Oy88oQ=Yhw@mail.gmail.com>
<CA+TgmoZ51Hc7o_vFo8rbPqkLpSVzH6i=ccPrt2mHBPyk=zMxwQ@mail.gmail.com>
<CAG0ozMq48+ODFoLW4HHexN8BKHfh_4c-n2b=yH9oaUnCZNZj6Q@mail.gmail.com>
<[email protected]>
<CAG0ozMpPhK1L5eUBq-FJU+uYTPoJ-EPfhPAHpyJvtJpBJHTcvQ@mail.gmail.com>
<CA+TgmoY0hj+P68iB_JrVMoEV4mbAKthiY2i36ksgFPfU7i80SQ@mail.gmail.com>
<CAG0ozMo=quwiEfU40n94NGqxB4DydKxNWyeD6U2OX68Z9RhSbg@mail.gmail.com>
<CA+TgmobGZseEpTeyhzstFs6rVKaB-_CfiC7gTuvz=QhWc=aR=w@mail.gmail.com>
<CAG0ozMovsrnnLGEM6h0puBr-ihLjYDp11kvnCrnym3D28BZ7eg@mail.gmail.com>
<CAG0ozMo2HQjmpn9AHGXOo726-PgH=+XmkoPUVxAqvcOS6puq8g@mail.gmail.com>
<CA+Tgmobjnj0FJj-0GUSXCga3-mUe819Wj=EeZqY=QydMWGOguA@mail.gmail.com>
<CAG0ozMpug5icxk8Mh_uiGc3OX3zsx67RsRA9Q-o+3ZXn-7M9mQ@mail.gmail.com>
<[email protected]>
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 <[email protected]> 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
view thread (43+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Proposal: Progressive explain
In-Reply-To: <CA+TgmobUpRO4j-cYzOHnDMuksZiRy87nPER2bkFamtpthmxoaw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox