From c4e4ee3c229a08f96f287ef34dbf06851e90604c Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 4 Jul 2024 12:53:21 +0100 Subject: [PATCH] Fix performance regression in `previous_version` function (#366) https://github.com/xataio/pgroll/pull/365 changed the definition of `previous_version` and introduced a performance regression. Update the function with extra `WHERE` clauses so that the recursive CTE can take better advantage of the table PK index. On a large (~1M record) `pgroll.migrations` table the query analysis for the recursive CTE is: **Before**: ``` +----------------------------------------------------------------------------------------------------------------------------------------------------------+ | QUERY PLAN | |----------------------------------------------------------------------------------------------------------------------------------------------------------| | CTE Scan on ancestors (cost=1819977.60..1819978.62 rows=51 width=214) (actual time=1163.608..6463.980 rows=1 loops=1) | | CTE ancestors | | -> Recursive Union (cost=0.00..1819977.60 rows=51 width=114) (actual time=1163.606..6463.977 rows=1 loops=1) | | -> Seq Scan on migrations (cost=0.00..442329.71 rows=1 width=114) (actual time=1163.604..6385.489 rows=1 loops=1) | | Filter: (name = latest_version('bb_00bo06t68d5ot7i5k5m8um70kg_bb19qc'::name)) | | Rows Removed by Filter: 1193140 | | -> Nested Loop (cost=0.55..137764.74 rows=5 width=114) (actual time=78.485..78.486 rows=0 loops=1) | | -> WorkTable Scan on ancestors a (cost=0.00..0.20 rows=10 width=36) (actual time=0.001..0.002 rows=1 loops=1) | | -> Index Scan using migrations_pkey on migrations m (cost=0.55..13776.44 rows=1 width=110) (actual time=78.459..78.459 rows=0 loops=1) | | Index Cond: (name = a.parent) | | Filter: ((migration_type)::text = 'inferred'::text) | | Rows Removed by Filter: 1 | | Planning Time: 0.764 ms | | JIT: | | Functions: 11 | | Options: Inlining true, Optimization true, Expressions true, Deforming true | | Timing: Generation 2.222 ms, Inlining 29.422 ms, Optimization 51.028 ms, Emission 24.552 ms, Total 107.225 ms | | Execution Time: 6466.347 ms | +----------------------------------------------------------------------------------------------------------------------------------------------------------+ ``` **After**: ``` +----------------------------------------------------------------------------------------------------------------------------------------------------+ | QUERY PLAN | |----------------------------------------------------------------------------------------------------------------------------------------------------| | CTE Scan on ancestors (cost=987.99..988.21 rows=11 width=214) (actual time=0.277..0.379 rows=1 loops=1) | | CTE ancestors | | -> Recursive Union (cost=0.55..987.99 rows=11 width=114) (actual time=0.275..0.377 rows=1 loops=1) | | -> Index Scan using history_is_linear on migrations (cost=0.55..128.36 rows=1 width=114) (actual time=0.274..0.354 rows=1 loops=1) | | Index Cond: (schema = 'bb_00bo06t68d5ot7i5k5m8um70kg_bb19qc'::name) | | Filter: (name = latest_version('bb_00bo06t68d5ot7i5k5m8um70kg_bb19qc'::name)) | | Rows Removed by Filter: 5 | | -> Nested Loop (cost=0.55..85.95 rows=1 width=114) (actual time=0.020..0.020 rows=0 loops=1) | | -> WorkTable Scan on ancestors a (cost=0.00..0.20 rows=10 width=100) (actual time=0.001..0.001 rows=1 loops=1) | | -> Index Scan using migrations_pkey on migrations m (cost=0.55..8.57 rows=1 width=110) (actual time=0.017..0.017 rows=0 loops=1) | | Index Cond: ((schema = a.schema) AND (name = a.parent)) | | Filter: ((migration_type)::text = 'inferred'::text) | | Rows Removed by Filter: 1 | | Planning Time: 0.744 ms | | Execution Time: 0.459 ms | +----------------------------------------------------------------------------------------------------------------------------------------------------+ ``` ie the recursive portion of the table is able to take advantage of the table's PK (defined on `(schema, name)`). --- pkg/state/state.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index d52abee1..2a9e26b2 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -75,14 +75,14 @@ STABLE; CREATE OR REPLACE FUNCTION %[1]s.previous_version(schemaname NAME) RETURNS text AS $$ WITH RECURSIVE ancestors AS ( - SELECT name, parent, migration_type, 0 AS depth FROM %[1]s.migrations - WHERE name = %[1]s.latest_version(schemaname) + SELECT name, schema, parent, migration_type, 0 AS depth FROM %[1]s.migrations + WHERE name = %[1]s.latest_version(schemaname) AND schema = schemaname UNION ALL - SELECT m.name, m.parent, m.migration_type, a.depth + 1 + SELECT m.name, m.schema, m.parent, m.migration_type, a.depth + 1 FROM %[1]s.migrations m - JOIN ancestors a ON m.name = a.parent + JOIN ancestors a ON m.name = a.parent AND m.schema = a.schema ) SELECT a.name FROM ancestors a JOIN information_schema.schemata s