postgres.git / summary / log / commit / refs

commit    baf17ad9dff4552b7e494d3f574972c21d9f90bc
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Fri Apr 24 16:02:36 2020 +0000
First released in: 13.0, 11.8, 12.3

    Repair performance regression in information_schema.triggers view.
    
    Commit 32ff26911 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


src/backend/catalog/information_schema.sql | 11 +++++++++-- src/include/catalog/catversion.h | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql index cb775683fa5..3e07fb107eb 100644 --- a/src/backend/catalog/information_schema.sql +++ b/src/backend/catalog/information_schema.sql @@ -2046,8 +2046,15 @@ CREATE VIEW triggers AS CAST( -- To determine action order, partition by schema, table, -- event_manipulation (INSERT/DELETE/UPDATE), ROW/STATEMENT (1), - -- BEFORE/AFTER (66), then order by trigger name - rank() OVER (PARTITION BY n.oid, c.oid, em.num, t.tgtype & 1, t.tgtype & 66 ORDER BY t.tgname) + -- BEFORE/AFTER (66), then order by trigger name. It's preferable + -- to partition by view output columns, so that query constraints + -- can be pushed down below the window function. + rank() OVER (PARTITION BY CAST(n.nspname AS sql_identifier), + CAST(c.relname AS sql_identifier), + em.num, + t.tgtype & 1, + t.tgtype & 66 + ORDER BY t.tgname) AS cardinal_number) AS action_order, CAST( CASE WHEN pg_has_role(c.relowner, 'USAGE') diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 13bbddf785e..09691d83578 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202004074 +#define CATALOG_VERSION_NO 202004241 #endif [parent: 4cac3a49e691]