v43-0001-Lock-table-first-when-setting-index-relation-sta.patch

From 40ec06c622c4be1de0e87509b2655e1d3ff62b4b Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huinker@gmail.com>
Date: Tue, 21 Jan 2025 11:52:58 -0500
Subject: [PATCH v43 1/2] Lock table first when setting index relation
 statistics.

Jian He reported [1] a missing lock relation bug in
pg_restore_relation_stats when restoring stats to an index.

This fix follows the steps for proper locking prior to an inplace update
of a relation as specified in aac2c9b4fde8, and mimics the locking
behavior of the analyze command, as well as correctly doing the ACL
checks against the underlying relation of an index rather than the index
itself.

These special rules do not apply to partitioned indexes, which have no
special case i check_inplace_rel_lock().

[1] https://www.postgresql.org/message-id/CACJufxGreTY7qsCV8%2BBkuv0p5SXGTScgh%3DD%2BDq6%3D%2B_%3DXTp7FWg%40mail.gmail.com
---
 src/backend/statistics/attribute_stats.c   |  2 +-
 src/backend/statistics/stat_utils.c        | 34 ++++++++++++--
 src/test/regress/expected/stats_import.out | 53 ++++++++++++++++++++++
 src/test/regress/sql/stats_import.sql      | 36 +++++++++++++++
 4 files changed, 119 insertions(+), 6 deletions(-)

diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index 94f7dd63a0..5d4762e404 100644
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -481,7 +481,7 @@ static Node *
 get_attr_expr(Relation rel, int attnum)
 {
 	if ((rel->rd_rel->relkind == RELKIND_INDEX
-		 || (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX))
+		|| (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX))
 		&& (rel->rd_indexprs != NIL)
 		&& (rel->rd_index->indkey.values[attnum - 1] == 0))
 	{
diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c
index 0d446f55b0..4af9c94174 100644
--- a/src/backend/statistics/stat_utils.c
+++ b/src/backend/statistics/stat_utils.c
@@ -18,12 +18,16 @@
 
 #include "access/relation.h"
 #include "catalog/pg_database.h"
+#include "catalog/index.h"
 #include "funcapi.h"
 #include "miscadmin.h"
 #include "statistics/stat_utils.h"
+#include "storage/lmgr.h"
+#include "storage/lockdefs.h"
 #include "utils/acl.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/rel.h"
 
 /*
@@ -126,14 +130,31 @@ stats_check_arg_pair(FunctionCallInfo fcinfo,
 void
 stats_lock_check_privileges(Oid reloid)
 {
-	Relation	rel = relation_open(reloid, ShareUpdateExclusiveLock);
-	const char	relkind = rel->rd_rel->relkind;
+	Relation	rel;
+	Oid			relation_oid = reloid;
+	Oid			index_oid = InvalidOid;
 
-	/* All of the types that can be used with ANALYZE, plus indexes */
-	switch (relkind)
+	/*
+	 * For indexes, we follow what do_analyze_rel() does so as to avoid any
+	 * deadlocks with analyze/vacuum, which is to take out a
+	 * ShareUpdateExclusive on table/matview first and then take an
+	 * AccessShareLock on the index itself. See check_inplace_rel_lock()
+	 * to see how this special case is implemented.
+	 */
+	if (get_rel_relkind(reloid) == RELKIND_INDEX)
+	{
+		relation_oid = IndexGetRelation(reloid, false);
+		index_oid = reloid;
+	}
+
+	rel = relation_open(relation_oid, ShareUpdateExclusiveLock);
+
+	/*
+	 * All of the types that can hold statistics, except for regular indexes
+	 */
+	switch (rel->rd_rel->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_INDEX:
 		case RELKIND_MATVIEW:
 		case RELKIND_FOREIGN_TABLE:
 		case RELKIND_PARTITIONED_TABLE:
@@ -164,6 +185,9 @@ stats_lock_check_privileges(Oid reloid)
 						   NameStr(rel->rd_rel->relname));
 	}
 
+	if (OidIsValid(index_oid))
+		LockRelationOid(index_oid, AccessShareLock);
+
 	relation_close(rel, NoLock);
 }
 
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index fb50da1cd8..2e80cce11d 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -85,6 +85,26 @@ WHERE oid = 'stats_import.test'::regclass;
        17 |       400 |             4
 (1 row)
 
+CREATE INDEX test_i ON stats_import.test(id);
+-- regular indexes have special case locking rules
+SELECT
+    pg_catalog.pg_set_relation_stats(
+        relation => 'stats_import.test_i'::regclass,
+        relpages => 18::integer);
+ pg_set_relation_stats 
+-----------------------
+ 
+(1 row)
+
+SELECT
+    pg_catalog.pg_restore_relation_stats(
+        'relation', 'stats_import.test_i'::regclass,
+        'relpages', 19::integer );
+ pg_restore_relation_stats 
+---------------------------
+ t
+(1 row)
+
 -- positional arguments
 SELECT
     pg_catalog.pg_set_relation_stats(
@@ -182,6 +202,7 @@ CREATE TABLE stats_import.part_child_1
   PARTITION OF stats_import.part_parent
   FOR VALUES FROM (0) TO (10)
   WITH (autovacuum_enabled = false);
+CREATE INDEX part_parent_i ON stats_import.part_parent(i);
 ANALYZE stats_import.part_parent;
 SELECT relpages
 FROM pg_class
@@ -202,6 +223,25 @@ SELECT
  
 (1 row)
 
+-- Partitioned indexes aren't analyzed but it is possible to set stats.
+SELECT
+    pg_catalog.pg_set_relation_stats(
+        relation => 'stats_import.part_parent_i'::regclass,
+        relpages => 2::integer);
+ pg_set_relation_stats 
+-----------------------
+ 
+(1 row)
+
+SELECT
+    pg_catalog.pg_restore_relation_stats(
+        'relation', 'stats_import.part_parent_i'::regclass,
+        'relpages', 2::integer);
+ pg_restore_relation_stats 
+---------------------------
+ t
+(1 row)
+
 -- nothing stops us from setting it to -1
 SELECT
     pg_catalog.pg_set_relation_stats(
@@ -1414,6 +1454,19 @@ SELECT 3, 'tre', (3, 3.3, 'TRE', '2003-03-03', NULL)::stats_import.complex_type,
 UNION ALL
 SELECT 4, 'four', NULL, int4range(0,100), NULL;
 CREATE INDEX is_odd ON stats_import.test(((comp).a % 2 = 1));
+/* Test for proper locking */
+SELECT * FROM pg_catalog.pg_restore_relation_stats(
+    'relation', 'stats_import.is_odd'::regclass,
+    'version', '180000'::integer,
+    'relpages', '11'::integer,
+    'reltuples', '10000'::real,
+    'relallvisible', '0'::integer
+);
+ pg_restore_relation_stats 
+---------------------------
+ t
+(1 row)
+
 -- Generate statistics on table with data
 ANALYZE stats_import.test;
 CREATE TABLE stats_import.test_clone ( LIKE stats_import.test )
diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index d3058bf8f6..d4d30a7368 100644
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -64,6 +64,19 @@ SELECT relpages, reltuples, relallvisible
 FROM pg_class
 WHERE oid = 'stats_import.test'::regclass;
 
+CREATE INDEX test_i ON stats_import.test(id);
+
+-- regular indexes have special case locking rules
+SELECT
+    pg_catalog.pg_set_relation_stats(
+        relation => 'stats_import.test_i'::regclass,
+        relpages => 18::integer);
+
+SELECT
+    pg_catalog.pg_restore_relation_stats(
+        'relation', 'stats_import.test_i'::regclass,
+        'relpages', 19::integer );
+
 -- positional arguments
 SELECT
     pg_catalog.pg_set_relation_stats(
@@ -127,6 +140,8 @@ CREATE TABLE stats_import.part_child_1
   FOR VALUES FROM (0) TO (10)
   WITH (autovacuum_enabled = false);
 
+CREATE INDEX part_parent_i ON stats_import.part_parent(i);
+
 ANALYZE stats_import.part_parent;
 
 SELECT relpages
@@ -140,6 +155,17 @@ SELECT
         relation => 'stats_import.part_parent'::regclass,
         relpages => 2::integer);
 
+-- Partitioned indexes aren't analyzed but it is possible to set stats.
+SELECT
+    pg_catalog.pg_set_relation_stats(
+        relation => 'stats_import.part_parent_i'::regclass,
+        relpages => 2::integer);
+
+SELECT
+    pg_catalog.pg_restore_relation_stats(
+        'relation', 'stats_import.part_parent_i'::regclass,
+        'relpages', 2::integer);
+
 -- nothing stops us from setting it to -1
 SELECT
     pg_catalog.pg_set_relation_stats(
@@ -1062,6 +1088,16 @@ SELECT 4, 'four', NULL, int4range(0,100), NULL;
 
 CREATE INDEX is_odd ON stats_import.test(((comp).a % 2 = 1));
 
+
+/* Test for proper locking */
+SELECT * FROM pg_catalog.pg_restore_relation_stats(
+    'relation', 'stats_import.is_odd'::regclass,
+    'version', '180000'::integer,
+    'relpages', '11'::integer,
+    'reltuples', '10000'::real,
+    'relallvisible', '0'::integer
+);
+
 -- Generate statistics on table with data
 ANALYZE stats_import.test;
 
-- 
2.48.1