From: | Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de>, Miles Delahunty <miles(dot)delahunty(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-bugs(at)lists(dot)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: can't drop table due to reference from orphaned temp function |
Date: | 2022-07-12 14:13:28 |
Message-ID: | 24a26615b66d84abf2083a10ff1ca59933a7b4e2.camel@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
Good day, Andres.
В Сб, 19/02/2022 в 13:31 -0800, Andres Freund пишет:
> Hi,
>
> On 2022-02-19 10:00:02 -0800, Andres Freund wrote:
> > See backtrace below [1]. The same problem does *not* exist when starting to
> > use the same temp schema in a new session (which drops the old contents
> > first), which kind of explains why we've not previously noticed this.
> >
> > But even so, I'm surprised we haven't noticed this before.
>
> Ah, there's a reason for that. In many cases we'll have a catalog snapshot
> registered, which is enough for init_toast_snapshot(). But in Miles' example,
> the object dropped just prior ends with catalog invalidations.
>
> Proposed bugfix and test attached.
>
> I think it's ok to backpatch the test. There might be a slight change in
> output due to 618c16707a6d6e8f5c83ede2092975e4670201ad not being backpatched,
> but that's OK I think.
>
>
> I think it is dangerous that we return a cached catalog snapshot for things
> like GetOldestSnapshot() unless they're also registered or active - we can't
> rely on catalog snapshots to be present. Indeed, if we didn't, this bug would
> have been found before, as some added assertions confirm.
>
> I don't think we can just ignore the catalog snapshot though, it can be
> registered in the future, so it actually is the oldest snapshot. But at least
> we should assert that there's some snapshot registered/active. In the attached
> patch I've added HaveRegisteredOrActiveSnapshot() and used that in
> init_toast_snapshot().
Reading your message and HaveRegisteredOrActiveSnapshot's body, I can't get its logic:
- if it is dangerous to have CatalogSnapshot alone in RegisteredSnapshots,
then why we return 'false' if RegisteredSnapshots is NOT singular?
I believe, body of HaveRegisteredOrActiveSnapshot should look like:
if (ActiveSnapshot != NULL)
return true;
if (pairingheap_is_empty(&RegisteredSnapshots))
return false;
/*
* The catalog snapshot is in RegisteredSnapshots when valid, but can be
* removed at any time due to invalidation processing. If explicitly
* registered more than one snapshot has to be in RegisteredSnapshots.
*/
if (pairingheap_is_singular(&RegisteredSnapshots))
return CatalogSnapshot == NULL;
return true;
Am I wrong?
regards,
Yura
From | Date | Subject | |
---|---|---|---|
Next Message | PG Bug reporting form | 2022-07-12 16:25:58 | BUG #17548: Aggregate queries on partitioned tables can cause OOM. |
Previous Message | Andrey Lepikhov | 2022-07-12 06:34:09 | Re: BUG #17544: Join order for INNER JOIN ... USING with GROUP BY on join column affects selected query plan |