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

Commit baf17ad

Browse files
committed
Repair performance regression in information_schema.triggers view.
Commit 32ff269 introduced use of rank() into the triggers view to calculate the spec-mandated action_order column. As written, this prevents query constraints on the table-name column from being pushed below the window aggregate step. That's bad for performance of this typical usage pattern, since the view now has to be evaluated for all tables not just the one(s) the user wants to see. It's also the cause of some recent buildfarm failures, in which trying to evaluate the view rows for triggers in process of being dropped resulted in "cache lookup failed for function NNN" errors. Those rows aren't of interest to the test script doing the query, but the filter that would eliminate them is being applied too late. None of this happened before the rank() call was there, so it's a regression compared to v10 and before. We can improve matters by changing the rank() call so that instead of partitioning by OIDs, it partitions by nspname and relname, casting those to sql_identifier so that they match the respective view output columns exactly. The planner has enough intelligence to know that constraints on partitioning columns are safe to push down, so this eliminates the performance problem and the regression test failure risk. We could make the other partitioning columns match view outputs as well, but it'd be more complicated and the performance benefits are questionable. Side note: as this stands, the planner will push down constraints on event_object_table and trigger_schema, but not on event_object_schema, because it checks for ressortgroupref matches not expression equivalence. That might be worth improving someday, but it's not necessary to fix the immediate concern. Back-patch to v11 where the rank() call was added. Ordinarily we'd not change information_schema in released branches, but the test failure has been seen in v12 and presumably could happen in v11 as well, so we need to do this to keep the buildfarm happy. The change is harmless so far as users are concerned. Some might wish to apply it to existing installations if performance of this type of query is of concern, but those who don't are no worse off. I bumped catversion in HEAD as a pro forma matter (there's no catalog incompatibility that would really require a re-initdb). Obviously that can't be done in the back branches. Discussion: https://postgr.es/m/5891.1587594470@sss.pgh.pa.us
1 parent 4cac3a4 commit baf17ad

File tree

2 files changed

+10
-3
lines changed

2 files changed

+10
-3
lines changed

src/backend/catalog/information_schema.sql

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2046,8 +2046,15 @@ CREATE VIEW triggers AS
20462046
CAST(
20472047
-- To determine action order, partition by schema, table,
20482048
-- event_manipulation (INSERT/DELETE/UPDATE), ROW/STATEMENT (1),
2049-
-- BEFORE/AFTER (66), then order by trigger name
2050-
rank() OVER (PARTITION BY n.oid, c.oid, em.num, t.tgtype & 1, t.tgtype & 66 ORDER BY t.tgname)
2049+
-- BEFORE/AFTER (66), then order by trigger name. It's preferable
2050+
-- to partition by view output columns, so that query constraints
2051+
-- can be pushed down below the window function.
2052+
rank() OVER (PARTITION BY CAST(n.nspname AS sql_identifier),
2053+
CAST(c.relname AS sql_identifier),
2054+
em.num,
2055+
t.tgtype & 1,
2056+
t.tgtype & 66
2057+
ORDER BY t.tgname)
20512058
AS cardinal_number) AS action_order,
20522059
CAST(
20532060
CASE WHEN pg_has_role(c.relowner, 'USAGE')

src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 202004074
56+
#define CATALOG_VERSION_NO 202004241
5757

5858
#endif

0 commit comments

Comments
 (0)