Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5b736e9

Browse files
committed
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
1 parent fc576b7 commit 5b736e9

File tree

1 file changed

+9
-11
lines changed

1 file changed

+9
-11
lines changed

src/backend/optimizer/plan/planner.c

+9-11
Original file line numberDiff line numberDiff line change
@@ -3056,14 +3056,16 @@ limit_needed(Query *parse)
30563056
* Since some other DBMSes do not allow references to ungrouped columns, it's
30573057
* not unusual to find all columns listed in GROUP BY even though listing the
30583058
* primary-key columns would be sufficient. Deleting such excess columns
3059-
* avoids redundant sorting work, so it's worth doing. When we do this, we
3060-
* must mark the plan as dependent on the pkey constraint (compare the
3061-
* parser's check_ungrouped_columns() and check_functional_grouping()).
3059+
* avoids redundant sorting work, so it's worth doing.
30623060
*
3063-
* In principle, we could treat any NOT-NULL columns appearing in a UNIQUE
3064-
* index as the determining columns. But as with check_functional_grouping(),
3065-
* there's currently no way to represent dependency on a NOT NULL constraint,
3066-
* so we consider only the pkey for now.
3061+
* Relcache invalidations will ensure that cached plans become invalidated
3062+
* when the underlying index of the pkey constraint is dropped.
3063+
*
3064+
* Currently, we only make use of pkey constraints for this, however, we may
3065+
* wish to take this further in the future and also use unique constraints
3066+
* which have NOT NULL columns. In that case, plan invalidation will still
3067+
* work since relations will receive a relcache invalidation when a NOT NULL
3068+
* constraint is dropped.
30673069
*/
30683070
static void
30693071
remove_useless_groupby_columns(PlannerInfo *root)
@@ -3172,10 +3174,6 @@ remove_useless_groupby_columns(PlannerInfo *root)
31723174

31733175
/* Remember the attnos of the removable columns */
31743176
surplusvars[relid] = bms_difference(relattnos, pkattnos);
3175-
3176-
/* Also, mark the resulting plan as dependent on this constraint */
3177-
parse->constraintDeps = lappend_oid(parse->constraintDeps,
3178-
constraintOid);
31793177
}
31803178
}
31813179

0 commit comments

Comments
 (0)