Skip to content

Commit d811ce6

Browse files
anarazelanarazel
committed
pgstat: Fix transactional stats dropping for indexes
Because index creation does not go through heap_create_with_catalog() we didn't call pgstat_create_relation(), leading to index stats of a newly created realtion not getting dropped during rollback. To fix, move the pgstat_create_relation() to heap_create(), which indexes do use. Similarly, because dropping an index does not go through heap_drop_with_catalog(), we didn't drop index stats when the transaction dropping an index committed. Here there's no convenient common path for indexes and relations, so index_drop() now calls pgstat_drop_relation(). Add tests for transactional index stats handling. Author: "Drouvot, Bertrand" <bdrouvot@amazon.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/51bbf286-2b4a-8998-bd12-eaae4b765d99@amazon.com Backpatch: 15-, like 8b1dccd, which introduced the bug
1 parent 0032a54 commit d811ce6

File tree

4 files changed

+161
-7
lines changed

4 files changed

+161
-7
lines changed

‎src/backend/catalog/heap.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,9 @@ heap_create(const char *relname,
403403
recordDependencyOnTablespace(RelationRelationId, relid,
404404
reltablespace);
405405

406+
/* ensure that stats are dropped if transaction aborts */
407+
pgstat_create_relation(rel);
408+
406409
return rel;
407410
}
408411

@@ -1477,9 +1480,6 @@ heap_create_with_catalog(const char *relname,
14771480
if (oncommit != ONCOMMIT_NOOP)
14781481
register_on_commit_action(relid, oncommit);
14791482

1480-
/* ensure that stats are dropped if transaction aborts */
1481-
pgstat_create_relation(new_rel_desc);
1482-
14831483
/*
14841484
* ok, the relation has been cataloged, so close our relations and return
14851485
* the OID of the newly created relation.

‎src/backend/catalog/index.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2325,6 +2325,9 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
23252325
if (RELKIND_HAS_STORAGE(userIndexRelation->rd_rel->relkind))
23262326
RelationDropStorage(userIndexRelation);
23272327

2328+
/* ensure that stats are dropped if transaction commits */
2329+
pgstat_drop_relation(userIndexRelation);
2330+
23282331
/*
23292332
* Close and flush the index's relcache entry, to ensure relcache doesn't
23302333
* try to rebuild it while we're deleting catalog entries. We keep the

‎src/test/regress/expected/stats.out

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ SET enable_indexscan TO on;
1818
SET enable_indexonlyscan TO off;
1919
-- not enabled by default, but we want to test it...
2020
SET track_functions TO 'all';
21+
-- record dboid for later use
22+
SELECT oid AS dboid from pg_database where datname = current_database() \gset
2123
-- save counters
2224
BEGIN;
2325
SET LOCAL stats_fetch_consistency = snapshot;
@@ -777,18 +779,121 @@ SELECT pg_stat_have_stats('bgwriter', 0, 0);
777779
SELECT pg_stat_have_stats('zaphod', 0, 0);
778780
ERROR: invalid statistics kind: "zaphod"
779781
-- db stats have objoid 0
780-
SELECT pg_stat_have_stats('database', (SELECT oid FROM pg_database WHERE datname = current_database()), 1);
782+
SELECT pg_stat_have_stats('database', :dboid, 1);
781783
pg_stat_have_stats
782784
--------------------
783785
f
784786
(1 row)
785787

786-
SELECT pg_stat_have_stats('database', (SELECT oid FROM pg_database WHERE datname = current_database()), 0);
788+
SELECT pg_stat_have_stats('database', :dboid, 0);
787789
pg_stat_have_stats
788790
--------------------
789791
t
790792
(1 row)
791793

794+
-- pg_stat_have_stats returns true for committed index creation
795+
CREATE table stats_test_tab1 as select generate_series(1,10) a;
796+
CREATE index stats_test_idx1 on stats_test_tab1(a);
797+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
798+
SET enable_seqscan TO off;
799+
select a from stats_test_tab1 where a = 3;
800+
a
801+
---
802+
3
803+
(1 row)
804+
805+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
806+
pg_stat_have_stats
807+
--------------------
808+
t
809+
(1 row)
810+
811+
-- pg_stat_have_stats returns false for dropped index with stats
812+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
813+
pg_stat_have_stats
814+
--------------------
815+
t
816+
(1 row)
817+
818+
DROP index stats_test_idx1;
819+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
820+
pg_stat_have_stats
821+
--------------------
822+
f
823+
(1 row)
824+
825+
-- pg_stat_have_stats returns false for rolled back index creation
826+
BEGIN;
827+
CREATE index stats_test_idx1 on stats_test_tab1(a);
828+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
829+
select a from stats_test_tab1 where a = 3;
830+
a
831+
---
832+
3
833+
(1 row)
834+
835+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
836+
pg_stat_have_stats
837+
--------------------
838+
t
839+
(1 row)
840+
841+
ROLLBACK;
842+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
843+
pg_stat_have_stats
844+
--------------------
845+
f
846+
(1 row)
847+
848+
-- pg_stat_have_stats returns true for reindex CONCURRENTLY
849+
CREATE index stats_test_idx1 on stats_test_tab1(a);
850+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
851+
select a from stats_test_tab1 where a = 3;
852+
a
853+
---
854+
3
855+
(1 row)
856+
857+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
858+
pg_stat_have_stats
859+
--------------------
860+
t
861+
(1 row)
862+
863+
REINDEX index CONCURRENTLY stats_test_idx1;
864+
-- false for previous oid
865+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
866+
pg_stat_have_stats
867+
--------------------
868+
f
869+
(1 row)
870+
871+
-- true for new oid
872+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
873+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
874+
pg_stat_have_stats
875+
--------------------
876+
t
877+
(1 row)
878+
879+
-- pg_stat_have_stats returns true for a rolled back drop index with stats
880+
BEGIN;
881+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
882+
pg_stat_have_stats
883+
--------------------
884+
t
885+
(1 row)
886+
887+
DROP index stats_test_idx1;
888+
ROLLBACK;
889+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
890+
pg_stat_have_stats
891+
--------------------
892+
t
893+
(1 row)
894+
895+
-- put enable_seqscan back to on
896+
SET enable_seqscan TO on;
792897
-- ensure that stats accessors handle NULL input correctly
793898
SELECT pg_stat_get_replication_slot(NULL);
794899
pg_stat_get_replication_slot

‎src/test/regress/sql/stats.sql

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ SET enable_indexonlyscan TO off;
1616
-- not enabled by default, but we want to test it...
1717
SET track_functions TO 'all';
1818

19+
-- record dboid for later use
20+
SELECT oid AS dboid from pg_database where datname = current_database() \gset
21+
1922
-- save counters
2023
BEGIN;
2124
SET LOCAL stats_fetch_consistency = snapshot;
@@ -388,9 +391,52 @@ SELECT pg_stat_have_stats('bgwriter', 0, 0);
388391
-- unknown stats kinds error out
389392
SELECT pg_stat_have_stats('zaphod', 0, 0);
390393
-- db stats have objoid 0
391-
SELECT pg_stat_have_stats('database', (SELECT oid FROM pg_database WHERE datname = current_database()), 1);
392-
SELECT pg_stat_have_stats('database', (SELECT oid FROM pg_database WHERE datname = current_database()), 0);
394+
SELECT pg_stat_have_stats('database', :dboid, 1);
395+
SELECT pg_stat_have_stats('database', :dboid, 0);
396+
397+
-- pg_stat_have_stats returns true for committed index creation
398+
CREATE table stats_test_tab1 as select generate_series(1,10) a;
399+
CREATE index stats_test_idx1 on stats_test_tab1(a);
400+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
401+
SET enable_seqscan TO off;
402+
select a from stats_test_tab1 where a = 3;
403+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
404+
405+
-- pg_stat_have_stats returns false for dropped index with stats
406+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
407+
DROP index stats_test_idx1;
408+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
409+
410+
-- pg_stat_have_stats returns false for rolled back index creation
411+
BEGIN;
412+
CREATE index stats_test_idx1 on stats_test_tab1(a);
413+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
414+
select a from stats_test_tab1 where a = 3;
415+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
416+
ROLLBACK;
417+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
418+
419+
-- pg_stat_have_stats returns true for reindex CONCURRENTLY
420+
CREATE index stats_test_idx1 on stats_test_tab1(a);
421+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
422+
select a from stats_test_tab1 where a = 3;
423+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
424+
REINDEX index CONCURRENTLY stats_test_idx1;
425+
-- false for previous oid
426+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
427+
-- true for new oid
428+
SELECT 'stats_test_idx1'::regclass::oid AS stats_test_idx1_oid \gset
429+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
430+
431+
-- pg_stat_have_stats returns true for a rolled back drop index with stats
432+
BEGIN;
433+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
434+
DROP index stats_test_idx1;
435+
ROLLBACK;
436+
SELECT pg_stat_have_stats('relation', :dboid, :stats_test_idx1_oid);
393437

438+
-- put enable_seqscan back to on
439+
SET enable_seqscan TO on;
394440

395441
-- ensure that stats accessors handle NULL input correctly
396442
SELECT pg_stat_get_replication_slot(NULL);

0 commit comments

Comments
 (0)