On Fri, 3 Apr 2020 at 21:56, Andy Fan <zhihui.fan1...@gmail.com> wrote: > > > > On Fri, Apr 3, 2020 at 12:08 PM David Rowley <dgrowle...@gmail.com> wrote: >> >> On Fri, 3 Apr 2020 at 16:40, Tom Lane <t...@sss.pgh.pa.us> wrote: >> > (It occurs to me BTW that we've been overly conservative about using >> > NOT NULL constraints in planning, because of failing to consider that. >> > Addition or drop of NOT NULL has to cause a change in >> > pg_attribute.attnotnull, which will definitely cause a relcache inval >> > on its table, cf rules in CacheInvalidateHeapTuple(). So we *don't* >> > need to have a pg_constraint entry corresponding to the NOT NULL, as >> > we've mistakenly supposed in some past discussions.) >> >> Agreed for remove_useless_groupby_columns(), but we'd need it if we >> wanted to detect functional dependencies in >> check_functional_grouping() using unique indexes. > > > Thanks for the explanation. I will add the removal in the next version of > this > patch.
There's no need for this patch to touch remove_useless_groupby_columns(). Fixes for that should be considered independently and *possibly* even backpatched.