The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation: not tested
Hi, this is a useful feature, thank you for implementing. I gather that it follows the standard, if so, then there are definitely no objections from me. The patch in version 2, applies cleanly and passes all the tests. It contains documentation which seems correct to a non native speaker. As a minor gripe, I would note the addition of list_int_cmp. The block + /* Sort each groupset individually */ + foreach(cell, result) + list_sort(lfirst(cell), list_int_cmp); Can follow suit from the rest of the code, and define a static cmp_list_int_asc(), as indeed the same patch does for cmp_list_len_contents_asc. This is indeed point of which I will not hold a too strong opinion. Overall :+1: from me. I will be bumping to 'Ready for Committer' unless objections.
