Lock table in DROP STATISTICS
authorTomas Vondra <[email protected]>
Sun, 19 Nov 2023 20:03:29 +0000 (21:03 +0100)
committerTomas Vondra <[email protected]>
Sun, 19 Nov 2023 20:03:38 +0000 (21:03 +0100)
The DROP STATISTICS code failed to properly lock the table, leading to

  ERROR:  tuple concurrently deleted

when executed concurrently with ANALYZE.

Fixed by modifying RemoveStatisticsById() to acquire the same lock as
ANALYZE. This function is called only by DROP STATISTICS, as ANALYZE
calls RemoveStatisticsDataById() directly.

Reported by Justin Pryzby, fix by me. Back through 12. The code was
like this since it was introduced in 10, but older releases are EOL.

Reported-by: Justin Pryzby
Reviewed-by: Tom Lane
Back-through: 12

Discussion: https://postgr.es/m/ZUuk-8CfbYeq6g_u@pryzbyj2023

src/backend/commands/statscmds.c

index 36bc8c33baf3f3c262169c03848803476d59ba09..20be564df276c37d83b57edd698b994fb214baa1 100644 (file)
@@ -734,18 +734,11 @@ void
 RemoveStatisticsById(Oid statsOid)
 {
        Relation        relation;
+       Relation        rel;
        HeapTuple       tup;
        Form_pg_statistic_ext statext;
        Oid                     relid;
 
-       /*
-        * First delete the pg_statistic_ext_data tuples holding the actual
-        * statistical data. There might be data with/without inheritance, so
-        * attempt deleting both.
-        */
-       RemoveStatisticsDataById(statsOid, true);
-       RemoveStatisticsDataById(statsOid, false);
-
        /*
         * Delete the pg_statistic_ext tuple.  Also send out a cache inval on the
         * associated table, so that dependent plans will be rebuilt.
@@ -760,12 +753,26 @@ RemoveStatisticsById(Oid statsOid)
        statext = (Form_pg_statistic_ext) GETSTRUCT(tup);
        relid = statext->stxrelid;
 
+       /*
+        * Delete the pg_statistic_ext_data tuples holding the actual statistical
+        * data. There might be data with/without inheritance, so attempt deleting
+        * both. We lock the user table first, to prevent other processes (e.g.
+        * DROP STATISTICS) from removing the row concurrently.
+        */
+       rel = table_open(relid, ShareUpdateExclusiveLock);
+
+       RemoveStatisticsDataById(statsOid, true);
+       RemoveStatisticsDataById(statsOid, false);
+
        CacheInvalidateRelcacheByRelid(relid);
 
        CatalogTupleDelete(relation, &tup->t_self);
 
        ReleaseSysCache(tup);
 
+       /* Keep lock until the end of the transaction. */
+       table_close(rel, NoLock);
+
        table_close(relation, RowExclusiveLock);
 }