On Oct 14, 2025, at 14:02, David Rowley <dgrowleyml@gmail.com> wrote:
When working on a5a68dd6d I noticed that truncate_useless_pathkeys()
uses a bunch of different static helper functions that are mostly the
same as each other. Most of them differ only in which field of
PlannerInfo they look at.
The attached aims to clean that up by making 2 reusable functions.
I've also fixed a few other things I noticed along the way.
Here's a list of what I've changed:
1. Add count_common_leading_pathkeys_ordered() function to check for
leading common pathkeys and use that for sort_pathkeys,
window_pathkeys and window_pathkeys.
2. Add count_common_leading_pathkeys_unordered() to check for leading
common pathkeys that exist in any portion of the other list of
pathkeys. Use this for group_pathkeys and distinct_pathkeys.
3. Add some short-circuiting to truncate_useless_pathkeys() as there's
no point in trying to trim down the list when some other operation has
already figured out that it needs all of the pathkeys.
4. Remove the stray " if (root->group_pathkeys != NIL) return true"
from has_useful_pathkeys().
<v1-0001-Tidyup-truncate_useless_pathkeys-function.patch>
I like 1 and 2 that make truncate_useless_pathkeys() easier to read. And I agree 3 is an optimization though I am not sure how much it will help.
For 4, I traced the function has_useful_pathkeys(), and it showed me that root->group_pathkeys and root->query_pathkeys actually point to the same list. So deletion of the check root->group_pathkeys is reasonable.
I have only a trivial comment. As you pull out the shared code into count_common_leading_pathkeys_ordered()/unordered(), it’s reasonable to make them inline, which ensures the new code has the same performance as before. However, I realized that truncate_useless_pathkeys() is not on a critical execution path, not making them inline won’t hurt very much. So, it’s up to you whether or not add “inline” to the two new functions.
Best reagards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
