David Rowley <dgrowle...@gmail.com> writes:
> 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.

Right.  I think it would be good for the comments to emphasize that
a relcache inval will be forced if the *index* underlying the pkey
constraint is dropped; the code doesn't care so much about the constraint
as such.  (This is also why it'd be safe to use a plain unique index
for the same optimization, assuming you can independently verify
non-nullness of the columns.  Maybe we should trash the existing coding
and just have it look for unique indexes + attnotnull flags.)

> 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.

+1 for removing the dependency and improving the comments in HEAD.
Minus quite a lot for back-patching: this is not a bug fix, and
there's a nonzero risk that we've overlooked something.  I'd rather
find that out in beta testing than from bug reports against stable
branches.

                        regards, tom lane


Reply via email to