Re: A tidyup of pathkeys.c

> On Oct 14, 2025, at 14:02, David Rowley <dgrowleyml(at)gmail(dot)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/