From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Mark Dilger <hornschnorter@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Missing dependency tracking for TableFunc nodes
Date: Thu, 14 Nov 2019 22:31:24 +0100
Message-ID: <20191114213124.i6xwx24y26iklzds@development> (raw)
In-Reply-To: <15228.1573695479@sss.pgh.pa.us>
References: <18427.1573508501@sss.pgh.pa.us>
<997cbcae-7739-f871-84a0-1192380abb54@gmail.com>
<15228.1573695479@sss.pgh.pa.us>
On Wed, Nov 13, 2019 at 08:37:59PM -0500, Tom Lane wrote:
>Mark Dilger <hornschnorter@gmail.com> writes:
>> On 11/11/19 1:41 PM, Tom Lane wrote:
>>> I happened to notice that find_expr_references_walker has not
>>> been taught anything about TableFunc nodes, which means it will
>>> miss the type and collation OIDs embedded in such a node.
>
>> I can consistently generate errors like the following in master:
>> ERROR: cache lookup failed for statistics object 31041
>
>This is surely a completely different issue --- there are not,
>one hopes, any extended-stats OIDs embedded in views or other
>query trees.
>
>I concur with Tomas' suspicion that this must be a race condition
>during DROP STATISTICS. If we're going to allow people to do that
>separately from dropping the table(s), there has to be some kind of
>locking around it, and it sounds like there's not :-(
>
I think the right thing to do is simply acquire AE lock on the relation
in RemoveStatisticsById, per the attached patch. It's possible we'll
need to do something more complicated once join stats are added, but
for now this should be enough (and backpatchable).
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:stats-locking.patch (text/plain, 1.0K) downloaddiff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.cindex 0ab43deb71..1126e7a585 100644--- a/src/backend/commands/statscmds.c+++ b/src/backend/commands/statscmds.c@@ -30,6 +30,7 @@
#include "commands/defrem.h"
#include "miscadmin.h"
#include "statistics/statistics.h"
+#include "storage/lmgr.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/inval.h"
@@ -533,6 +534,21 @@ RemoveStatisticsById(Oid statsOid)
Form_pg_statistic_ext statext;
Oid relid;
+ /* Lookup and lock relation the statistics is defined on. */+ tup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statsOid));++ if (!HeapTupleIsValid(tup))+ elog(ERROR, "cache lookup failed for statistics object %u", statsOid);++ statext = (Form_pg_statistic_ext) GETSTRUCT(tup);+ relid = statext->stxrelid;++ /* lock the relation in access exclusive mode */+ LockRelationOid(relid, AccessExclusiveLock);++ ReleaseSysCache(tup);++
/*
* First delete the pg_statistic_ext_data tuple holding the actual
* statistical data.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, tgl@sss.pgh.pa.us, hornschnorter@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Missing dependency tracking for TableFunc nodes
In-Reply-To: <20191114213124.i6xwx24y26iklzds@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox