> On Oct 14, 2025, at 14:02, David Rowley <[email protected]> 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/
