Re: Remove useless GROUP BY columns considering unique index

On Thu, 28 Nov 2024 at 17:39, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> v4 logic is to choose one with the least number of columns.
> if there is more than one with the least number of columns, simply
> choose the first one
> in the matched list.

The new code inside remove_useless_groupby_columns() is still more
complex than what's needed. There's no need to build a 2nd list with
the matching indexes and loop over it. Just skip the non-matching
indexes and record the best match.

I imagined it would look a bit more like the attached patch. I also
removed the pk_pos column as I didn't see any point in it. You were
not using it. If some future code needs that, it can be added then. I
also fixed up a few comments that needed attention. I didn't look at
the regression tests being added. I think given that there's now no
special case for primary key indexes that we don't need a completely
new set of tests. I think it'll make more sense to adjust some of the
existing tests to use a unique constraint instead of a PK and then
adjust a column's NOT NULL property to check that part of the code is
working correctly.

Another issue with this is that the list of indexes being used is not
sorted by Oid. If you look at RelationGetIndexList() you'll see that
we perform a sort. That gives us more consistency in the planner. I
think this patch should be doing that too, otherwise, you could end up
with a plan change after some operation that changes the order that
the indexes are stored in the pg_index table. It's probably fairly
unlikely, but it is the sort of issue that someone will eventually
discover and report.

David