Thread: [PATCH] Include triggers in EXPLAIN

[PATCH] Include triggers in EXPLAIN

From
Josef Šimánek
Date:
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 https://www.postgresql.org/message-id/flat/20693.1111732761%40sss.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 https://www.postgresql.org/message-id/21221.1111736869%40sss.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
Attachment

Re: [PATCH] Include triggers in EXPLAIN

From
Tom Lane
Date:
=?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



Re: [PATCH] Include triggers in EXPLAIN

From
Josef Šimánek
Date:
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:
Josef Šimánek <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

Re: [PATCH] Include triggers in EXPLAIN

From
Andres Freund
Date:
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