Wrong results in remove_useless_groupby_columns()
I seem to have fallen into a rabbit-hole around equality-relation
issues. I noticed another wrong-result issue caused by collation
mismatch. This time it is in remove_useless_groupby_columns().
remove_useless_groupby_columns() uses a relation's unique indexes to
prove that some GROUP BY columns are functionally dependent on others,
and so can be dropped from the GROUP BY clause. The match between
index columns and GROUP BY columns is done by attno alone.
However, a unique index under one collation does not prove uniqueness
under another. For example:
create collation ci (provider = icu, locale = 'und-u-ks-level2',
deterministic = false);
create table t (a text collate ci not null, b text);
insert into t values ('foo','X'), ('FOO','Y');
create unique index on t (a collate "C");
-- wrong results: should be 2 rows
select a, b from t group by a, b;
a | b
-----+---
foo | X
(1 row)
The wrong-result is caused by the planner incorrectly dropping b from
GROUP BY.
What is more, remove_useless_groupby_columns() does not bother to
check opfamily either. A unique index under one opfamily does not
prove uniqueness under the equality used by GROUP BY when the
SortGroupClause's eqop comes from a different opfamily. For example:
create type myrec as (x numeric);
create table opf_t (a myrec not null, b text);
create unique index on opf_t (a record_image_ops);
insert into opf_t values (row(1.0)::myrec, 'X'), (row(1.00)::myrec, 'Y');
-- wrong results: should be 2 rows
select a, b from opf_t group by a, b;
a | b
-------+---
(1.0) | X
(1 row)
ROW(1.0) and ROW(1.00) are bytewise distinct, so record_image_ops
admits both rows. But GROUP BY a uses the type's default record_ops,
under which they are logically equal, so the two rows merge into one
group. And again, the planner drops b and a row is silently lost.
Attached patch fixes both issues.
- Richard
Attachments:
v1-0001-Consider-opfamily-and-collation-when-removing-red.patchapplication/octet-stream; name=v1-0001-Consider-opfamily-and-collation-when-removing-red.patchDownload+160-9
On Tue, May 5, 2026 at 7:12 PM Richard Guo <guofenglinux@gmail.com> wrote:
Attached patch fixes both issues.
I plan to push and back-patch this shortly, as the code freeze for the
stable branches is just around the corner.
- Richard
Hi,
On Fri, 8 May 2026 at 05:46, Richard Guo <guofenglinux@gmail.com> wrote:
On Tue, May 5, 2026 at 7:12 PM Richard Guo <guofenglinux@gmail.com> wrote:
Attached patch fixes both issues.
I plan to push and back-patch this shortly, as the code freeze for the
stable branches is just around the corner.- Richard
I reviewed the patch and it looks good to me.
The added opfamily and collation checks seem consistent with the uniqueness
proof used in relation_has_unique_index_for(), and the tests cover both
reported wrong-result cases.
Applied the patch, ran tests, they passed.
Regards,
Ayush
On Fri, May 8, 2026 at 11:57 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:
On Fri, 8 May 2026 at 05:46, Richard Guo <guofenglinux@gmail.com> wrote:
I plan to push and back-patch this shortly, as the code freeze for the
stable branches is just around the corner.
I reviewed the patch and it looks good to me.
The added opfamily and collation checks seem consistent with the uniqueness
proof used in relation_has_unique_index_for(), and the tests cover both
reported wrong-result cases.Applied the patch, ran tests, they passed.
Thanks for reviewing. I've committed this and back-patched it to v18.
I initially thought it needed to go back to v14, but it turns out to
be a v18 regression. Before v18, remove_useless_groupby_columns()
consulted only primary keys, whose enforcement index is required by
parse_utilcmd.c to use the default opclass and the column's declared
collation, so neither mismatch could arise.
- Richard