> After the introduction of pg_overexplain extension and Robert's comment
> [0], I'm starting to have doubts about whether it's still appropriate to
> add this information to EXPLAIN itself. If there are compelling reasons
> that showing the plan type would be broadly useful to users in EXPLAIN,
>
rebased in the attached v5.
--
Sami Imseih
Amazon Web Services (AWS)
v5-0001-Add-plan_cache-counters-to-pg_stat_statements.patch
Description: Binary data
After the introduction of pg_overexplain extension and Robert's comment
[0], I'm starting to have doubts about whether it's still appropriate to
add this information to EXPLAIN itself. If there are compelling reasons
that showing the plan type would be broadly useful to users in EXPLAIN,
I’m ha
> Thank you for your patch. It is really useful for tracking the history
> of generic and custom plan usage.
Thanks for the review!
> 1. Is there any reason for the double check of cplan != NULL? It seems
> unnecessary, and we could simplify it to:
>
> -if (cplan && cplan->status == PLAN_CACHE_ST
> I don't quite understand why do we need to differentiate between
> PLAN_CACHE_STATUS_GENERIC_PLAN_BUILD and
> PLAN_CACHE_STATUS_GENERIC_PLAN_REUSE?
> We could simply keep PLAN_CACHE_STATUS_GENERIC_PLAN_REUSE.
> I don't think users would see much of a difference in either
> pg_stat_statements or
On 06.03.2025 18:04, Sami Imseih wrote:
2. Should we add Assert(kind == PGSS_EXEC) at this place to ensure that
generic_plan_calls and custom_plan_calls are only incremented when
appropriate?
I don't think an assert is needed here. There is an assert at the start of
the block for PGSS_EXEC an
Hi,
Thank you for your patch. It is really useful for tracking the history
of generic and custom plan usage.
At first glance, I have the following suggestions for improvement:
1. Is there any reason for the double check of cplan != NULL? It seems
unnecessary, and we could simplify it to:
-
>
> Please see v2
>
oops, had to fix a typo in meson.build. Please see v3.
--
Sami
v3-0001-add-plan_cache-counters-to-pg_stat_statements.patch
Description: Binary data
Thanks for the review!
> I could see EXPLAIN being somewhat useful (especially for non-interactive
> things like auto_explain), so a weak +1 on that.
I'll make this a follow-up to this patch.
> Definitely not useful for pg_stat_database IMHO.
agree as well. I did not have strong feelings about
Overall I like the idea; adds some nice visibility to something that has
been very ephemeral in the past.
Not included in this patch and maybe for follow-up work, is this
> information a good idea also?
can be added to EXPLAIN output and perhaps pg_stat_database.
>
I could see EXPLAIN being some
10 matches
Mail list logo
