Hi! On Sun, Jun 2, 2024 at 10:55 AM jian he <jian.universal...@gmail.com> wrote: > > On Fri, May 31, 2024 at 8:12 AM Alexander Korotkov <aekorot...@gmail.com> > wrote: > > > > I've revised some grammar including the sentence you've proposed. > > > > -static List *groupclause_apply_groupingset(PlannerInfo *root, List *force); > +static List *preprocess_groupclause(PlannerInfo *root, List *force); > > changing preprocess_groupclause the second argument > from "force" to "gset" would be more intuitive, I think.
Probably, but my intention is to restore preprocess_groupclause() as it was before 0452b461bc with minimal edits to support incremental sort. I'd rather avoid refactoring if this area for now. > `elog(ERROR, "Order of group-by clauses doesn't correspond incoming > sort order");` > > I think this error message makes people wonder what "incoming sort order" is. > BTW, "correspond", generally people use "correspond to". Thank you. On the second thought, I think it would be better to turn this into an assertion like the checks before. > I did some minor cosmetic changes, mainly changing foreach to foreach_node. > Please check the attachment. I would avoid refactoring of preprocess_groupclause() for the reason described above. But I picked the grammar fix for PlannerInfo's comment. ------ Regards, Alexander Korotkov Supabase
v4-0003-Rename-PathKeyInfo-to-GroupByOrdering.patch
Description: Binary data
v4-0004-Restore-preprocess_groupclause.patch
Description: Binary data
v4-0005-Teach-group_keys_reorder_by_pathkeys-about-redund.patch
Description: Binary data
v4-0001-Fix-asymmetry-in-setting-EquivalenceClass.ec_sort.patch
Description: Binary data
v4-0002-Add-invariants-check-to-get_useful_group_keys_ord.patch
Description: Binary data
