Re: Parallel leader process info in EXPLAIN
От | Melanie Plageman |
---|---|
Тема | Re: Parallel leader process info in EXPLAIN |
Дата | |
Msg-id | CAAKRu_YtmwgdWmy_8_CiZMJDVF65HP15qB6=EUSv_wL1zx35CA@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Parallel leader process info in EXPLAIN (Melanie Plageman <melanieplageman@gmail.com>) |
Ответы |
Re: Parallel leader process info in EXPLAIN
|
Список | pgsql-hackers |
So, I think from a code review perspective the code in the patches
LGTM.
As for the EXPLAIN ANALYZE tests--I don't see that many of them in
regress, so maybe that's because they aren't normally very useful. In
this case, it would only be to protect against regressions in printing
the leader instrumentation, I think.
The problem with that is, even with all the non-deterministic info
redacted, if the leader doesn't participate (which is not guaranteed),
then its stats wouldn't be printed at all and that would cause an
incorrectly failing test case...okay I just talked myself out of the
usefulness of testing this.
So, I would move it to "ready for committer", but, since it is not
applying cleanly, I have changed the status to "waiting on author".
LGTM.
As for the EXPLAIN ANALYZE tests--I don't see that many of them in
regress, so maybe that's because they aren't normally very useful. In
this case, it would only be to protect against regressions in printing
the leader instrumentation, I think.
The problem with that is, even with all the non-deterministic info
redacted, if the leader doesn't participate (which is not guaranteed),
then its stats wouldn't be printed at all and that would cause an
incorrectly failing test case...okay I just talked myself out of the
usefulness of testing this.
So, I would move it to "ready for committer", but, since it is not
applying cleanly, I have changed the status to "waiting on author".
В списке pgsql-hackers по дате отправления: