[PATCH] Include triggers in EXPLAIN
Hello!
Recently I got few times into situation where I was trying to find out what
is blocking DELETE queries. Running EXPLAIN (even VERBOSE one) wasn't
useful, since the reason was slow trigger (missing index on foreign key
column). I had to create testing entry and run EXPLAIN ANALYZE DELETE to
get this information.
It will be really valuable for me to show triggers in EXPLAIN query since
it will make clear for me there will be any trigger "activated" during
execution of DELETE query and that can be the reason for slow DELETE.
I have seen initial discussion at
/messages/by-id/20693.1111732761@sss.pgh.pa.us
to show time spent in triggers in EXPLAIN ANALYZE including quick
discussion to possibly show triggers during EXPLAIN. Anyway since it
doesn't show any additional cost and just inform about the possibilities, I
still consider this feature useful. This is probably implementation of idea
mentioned at
/messages/by-id/21221.1111736869@sss.pgh.pa.us by
Tom Lane.
After initial discussion with Pavel Stěhule and Tomáš Vondra at czech
postgresql maillist (
https://groups.google.com/forum/#!topic/postgresql-cz/Dq1sT7huVho) I was
able to prepare initial version of this patch. I have added EXPLAIN option
called TRIGGERS enabled by default.There's already autoexplain property for
this. I understand it is not possible to show only triggers which will be
really activated unless query is really executed. EXPLAIN ANALYZE remains
untouched with this patch.
- patch with examples can be found at
https://github.com/simi/postgres/pull/2
- DIFF format https://github.com/simi/postgres/pull/2.diff
- PATCH format (also attached) https://github.com/simi/postgres/pull/2.patch
All regression tests passed with this change locally on latest git master.
I would like to cover this patch with more regression tests, but I wasn't
sure where to place them, since there's no "EXPLAIN" related test "group".
Is "src/test/regress/sql/triggers.sql" the best place to add tests related
to this change?
PS: This is my first try to contribute to postgresql codebase. The quality
of patch is probably not the best, but I will be more than happy to do any
requested change if needed.
Regards,
Josef Šimánek
Attachments:
explain-triggers.patchtext/x-patch; charset=US-ASCII; name=explain-triggers.patchDownload+60-13
=?UTF-8?B?Sm9zZWYgxaBpbcOhbmVr?= <josef.simanek@gmail.com> writes:
Recently I got few times into situation where I was trying to find out what
is blocking DELETE queries. Running EXPLAIN (even VERBOSE one) wasn't
useful, since the reason was slow trigger (missing index on foreign key
column). I had to create testing entry and run EXPLAIN ANALYZE DELETE to
get this information.
It will be really valuable for me to show triggers in EXPLAIN query since
it will make clear for me there will be any trigger "activated" during
execution of DELETE query and that can be the reason for slow DELETE.
I don't really see the point of this patch? You do get the trigger
times during EXPLAIN ANALYZE, and I don't believe that a plain EXPLAIN
is going to have full information about what triggers might fire or
not fire.
regards, tom lane
Hello Tom.
Thanks for quick response. As I was testing this feature it shows all
"possible" triggers to be executed running given query. The benefit of
having this information in EXPLAIN as well is you do not need to execute
the query (as EXPLAIN ANALYZE does). My usecase is to take a look at query
before it is executed to get some idea about the plan with EXPLAIN.
Do you have idea about some case where actual trigger will be missing in
EXPLAIN with current implementation, but will be present in EXPLAIN
ANALYZE? I can take a look if there's any way how to handle those cases as
well.
ne 3. 11. 2019 v 22:49 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Show quoted text
=?UTF-8?B?Sm9zZWYgxaBpbcOhbmVr?= <josef.simanek@gmail.com> writes:
Recently I got few times into situation where I was trying to find out
what
is blocking DELETE queries. Running EXPLAIN (even VERBOSE one) wasn't
useful, since the reason was slow trigger (missing index on foreign key
column). I had to create testing entry and run EXPLAIN ANALYZE DELETE to
get this information.It will be really valuable for me to show triggers in EXPLAIN query since
it will make clear for me there will be any trigger "activated" during
execution of DELETE query and that can be the reason for slow DELETE.I don't really see the point of this patch? You do get the trigger
times during EXPLAIN ANALYZE, and I don't believe that a plain EXPLAIN
is going to have full information about what triggers might fire or
not fire.regards, tom lane
Hi,
(minor note - on PG lists the style is to quote in-line and trip)
On 2019-11-04 10:35:25 +0100, Josef Šimánek wrote:
Thanks for quick response. As I was testing this feature it shows all
"possible" triggers to be executed running given query. The benefit of
having this information in EXPLAIN as well is you do not need to execute
the query (as EXPLAIN ANALYZE does). My usecase is to take a look at query
before it is executed to get some idea about the plan with EXPLAIN.
I can actually see some value in additional information here, but I'd
probably want to change the format a bit. When explicitly desired (or
perhaps just in verbose mode?), I see value in counting the number of
triggers we know about that need to be checked, how many were excluded
on the basis of the trigger's WHEN clause etc.
Do you have idea about some case where actual trigger will be missing in
EXPLAIN with current implementation, but will be present in EXPLAIN
ANALYZE? I can take a look if there's any way how to handle those cases as
well.
Any triggers that are fired because of other, listed, triggers causing
other changes. E.g. a logging trigger that inserts into a log table -
EXPLAIN, without ANALYZE, doesn't have a way of knowing about that.
And before you say that sounds like a niche issue - it's not in my
experience. Forgetting the necessary indexes two or three foreign keys
down a CASCADE chain seems to be more common than doing so for tables
directly "linked" with busy ones.
Greetings,
Andres Freund