Re: proposal - queryid can be used as filter for auto_explain

Hi

st 1. 7. 2026 v 0:19 odesílatel Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
napsal:

> Thanks, the changes look good to me, I only have some minor comments
> about naming/grammar:
>
> +
> +static auto_explain_queryids *queryId_filter = NULL;
> +
>

That queryId_filter mixed case name seems strange, maybe
> queryid_filter would be better?
>

changed

Unfortunately, there is not name consistency already for queryid (without
this patch) - somewhere is queryid elsewhere is queryId

>
> + /*
> + * We expect small number of watched
> queryids, and then
> + * a linear seaching is the fastest. As an
> alternative
> + * we can sort the array of queryId, and
> we can search
> + * there by bisection.
> + */
>
> The patch should do either of those, but I don't think this comment
> should stay in it after that decision? Linear search seems to be the
> simpler/better choice to me, unless the goal is to handle 32-64-or
> more ids.
>

ok, I removed this comment

>
> + /* Try to allocate an auto_explain_extension_options object. */
> + allocsize = offsetof(auto_explain_queryids, queryId) +
> + sizeof(int64) * list_length(elemlist);
>
> The type is auto_explain_queryids, auto_explain_extension_options
> isn't mentioned anywhere else?
>

yes - it is copy/paste error, fixed

>
> "allows to specify list of queryid" - maybe query IDs?
> "Only plan of query with queryid" - same
> "Only queries with queryId from list will be logged." - same issue,
> maybe "Only queries with the listed IDs will be logged?"
> "works together" - work together
>

changed

Thank you for review