| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: proposal - queryid can be used as filter for auto_explain |
| Date: | 2026-06-30 22:19:32 |
| Message-ID: | CAN4CZFPE_JGAAmJCf_xEjOhEeLbSHrj5X33avBhDDvZKeFOZmQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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?
+ /*
+ * 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.
+ /* 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?
"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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-06-30 22:36:57 | Re: Add pg_stat_kind_info system view |
| Previous Message | Chao Li | 2026-06-30 22:08:24 | Remove radius from initdb authentication methods |