From 5b736e9cf95793ca6a4c00d291a06dfe3559c8ec Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 17 Apr 2020 10:29:49 +1200 Subject: [PATCH] Remove unneeded constraint dependency tracking It was previously thought that remove_useless_groupby_columns() needed to keep track of which constraints the generated plan depended upon, however, this is unnecessary. The confusion likely arose regarding this because of check_functional_grouping(), which does need to track the dependency to ensure VIEWs with columns which are functionally dependant on the GROUP BY remain so. For remove_useless_groupby_columns(), cached plans will just become invalidated when the primary key's underlying index is removed through the normal relcache invalidation code. Here we just remove the unneeded code which records the dependency and updates the comments. The previous comments claimed that we could not use UNIQUE constraints for the same optimization due to lack of a pg_constraint record for NOT NULL constraints (which are required because NULLs can be duplicated in a unique index). Since we don't actually need a pg_constraint record to handle the invalidation, it looks like we could add code to do this in the future. But not today. We're not really fixing any bug in the code here, this fix is just to set the record straight on UNIQUE constraints. This code was added back in 9.6, but due to lack of any bug, we'll not be backpatching this. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAApHDvrdYa=VhOoMe4ZZjZ-G4ALnD-xuAeUNCRTL+PYMVN8OnQ@mail.gmail.com --- src/backend/optimizer/plan/planner.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index b31c52404a..e664eb18c0 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3056,14 +3056,16 @@ limit_needed(Query *parse) * Since some other DBMSes do not allow references to ungrouped columns, it's * not unusual to find all columns listed in GROUP BY even though listing the * primary-key columns would be sufficient. Deleting such excess columns - * avoids redundant sorting work, so it's worth doing. When we do this, we - * must mark the plan as dependent on the pkey constraint (compare the - * parser's check_ungrouped_columns() and check_functional_grouping()). + * avoids redundant sorting work, so it's worth doing. * - * In principle, we could treat any NOT-NULL columns appearing in a UNIQUE - * index as the determining columns. But as with check_functional_grouping(), - * there's currently no way to represent dependency on a NOT NULL constraint, - * so we consider only the pkey for now. + * Relcache invalidations will ensure that cached plans become invalidated + * when the underlying index of the pkey constraint is dropped. + * + * Currently, we only make use of pkey constraints for this, however, we may + * wish to take this further in the future and also use unique constraints + * which have NOT NULL columns. In that case, plan invalidation will still + * work since relations will receive a relcache invalidation when a NOT NULL + * constraint is dropped. */ static void remove_useless_groupby_columns(PlannerInfo *root) @@ -3172,10 +3174,6 @@ remove_useless_groupby_columns(PlannerInfo *root) /* Remember the attnos of the removable columns */ surplusvars[relid] = bms_difference(relattnos, pkattnos); - - /* Also, mark the resulting plan as dependent on this constraint */ - parse->constraintDeps = lappend_oid(parse->constraintDeps, - constraintOid); } }