From: | Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Logging query parmeters in auto_explain |
Date: | 2022-06-27 11:22:42 |
Message-ID: | 87iloml5a5.fsf@wibble.ilmari.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Thu, Jun 09, 2022 at 11:55:11PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Done (and more tests added), v3 attached.
>
> One thing I am wondering is if we'd better mention errdetail_params()
> at the top of the initial check done in ExplainQueryParameters().
> That's a minor issue, though.
>
> +sub query_log
> +{
> Perhaps a short description at the top of this routine to explain it
> is worth it?
Done. I also moved the function to the bottom of the file, to avoid
distracting from the actual test queries.
> The test still does a set of like() and unlike() after running each
> query when the parameter updates are done. One thing I would have
> played with is to group the set of logs expected or not expected as
> parameters of the centralized routine, but that would reduce the
> customization of the test names, so at the end the approach you have
> taken for query_log() looks good to me.
I did consider passing the tests as a data structure to the function,
but that would amount to specifying exactly the same things but as a
data structure, and then calling the appropriate function by reference,
which just makes things more cluttered.
If we were using TAP subtests, it might make a sense to have the
function run each set of related tests in a subtest, rather than having
multiple subtest calls at the top level, but we don't, so it doesn't.
> +$node->stop('fast');
> There is no need for that. The END block of Cluster.pm does that
> already.
Ah, I was not aware of that. The call was there in the original version,
so I had just left it in. Removed.
v4 patch attached.
- ilmari
Attachment | Content-Type | Size |
---|---|---|
v4-0001-Log-query-parameters-in-auto_explain.patch | text/x-diff | 9.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dagfinn Ilmari Mannsåker | 2022-06-27 11:27:57 | Re: Logging query parmeters in auto_explain |
Previous Message | John Naylor | 2022-06-27 11:12:13 | Re: [PoC] Improve dead tuple storage for lazy vacuum |