On Thu, Oct 5, 2023 at 2:26 PM David Rowley <dgrowle...@gmail.com> wrote:
> So in short, I propose the attached fix without any regression tests > because I feel that any regression test would just mark that there was > a big in create_agg_path() and not really help with ensuring we don't > end up with some similar problem in the future. If the pathkeys that were added by adjust_group_pathkeys_for_groupagg() are computable from the targetlist, it seems that we do not need to trim them off, because prepare_sort_from_pathkeys() will add resjunk target entries for them. But it's also no harm if we trim them off. So I think the patch is a pretty safe fix. +1 to it. > I have some concerns that the assert_pathkeys_in_target() function > might be a little heavyweight for USE_ASSERT_CHECKING builds. So I'm > not proposing to commit that without further discussion. Yeah, it looks like some heavy to call assert_pathkeys_in_target() for each path node. Can we run some benchmarks to see how much overhead it would bring to USE_ASSERT_CHECKING build? Thanks Richard
