On Tue, Apr 1, 2025 at 3:05 PM Sami Imseih <samimseih@gmail.com> wrote:
> > Looking at ExplainAssembleLogOutput() is making me realize that
> > auto_explain is in serious need of some cleanup. That's not really the
> > fault of this patch, but the hack whereby we overwrite the [] that
> > would have surrounded the JSON output with {} is not very nice. I also
> > think that the auto_explain GUCs need rethinking. In theory, new
> > EXPLAIN options should be mirrored into auto_explain, but if you
> > compare ExplainAssembleLogOutput() to ExplainOnePlan(), you can see
> > that they are diverging. The PLANNING, SUMMARY, and SERIALIZE options
> > that are known to regular EXPLAIN aren't known to auto_explain, and
> > any customizable options that use explain_per_plan_hook won't be able
> > to work with auto_explain, either. Changing this is non-trivial
> > because SERIALIZE, for example, can't work the same way for
> > auto_explain as it does for EXPLAIN, and a full solution might also
> > require user-visible changes like replacing
> > auto_explain.log_<whatever> with auto_explain.explain, so I don't
> > really know. Maybe we should just live with it the way it is for now,
> > but it doesn't look very nice.
>
> FWIW, I have been thinking about auto_explain for another task,
> remote plans for fdw [0], and perhaps there are now other good
> reasons, some that you mention, that can be simplified if "auto_explain"
> becomes a core feature. This could be a proposal taken up in 19.
For what we're talking about here, I don't think we would need to go
that far -- maybe put a few functions in core but no real need to move
the whole module into core. However, I don't rule out that there are
other reasons to do as you suggest.
--
Robert Haas
EDB: http://www.enterprisedb.com