Hi,
Over in [1], Tom and I had a discussion in response to some confusion
about why remove_useless_groupby_columns() goes to the trouble of
recording a dependency on the PRIMARY KEY constraint when removing
surplus columns from the GROUP BY clause.
The outcome was that we don't need to do this since
remove_useless_groupby_columns() is used only as a plan-time
optimisation, we don't need to record any dependency. Unlike
check_functional_grouping(), where we must record the dependency as we
may end up with a VIEW with columns, e.g, in the select list which are
functionally dependant on a pkey constraint. In that case, we must
ensure the view is also removed, or that the constraint removal is
blocked. There's no such requirement for planner smarts, such as the
one in remove_useless_groupby_columns() as in that case we'll trigger
a relcache invalidation during ALTER TABLE DROP CONSTRAINT, which
cached plans will notice when they obtain their locks just before
execution begins.
To prevent future confusion, I'd like to remove dependency recording
code from remove_useless_groupby_columns() and update the misleading
comment. Likely this should also be backpatched to 9.6.
Does anyone think differently?
A patch to do this is attached.
[1] https://www.postgresql.org/message-id/CAApHDvr4OW_OUd_Rxp0d1hRgz+a4mm8+8uR7QoM2VqKFX08SqA@mail.gmail.com
