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+8ur7qom2vqkfx08...@mail.gmail.com
fix_remove_useless_groupby_columns.patch
Description: Binary data
