Thanks!
At Sun, 26 Jan 2020 20:22:01 -0800, Noah Misch <noah@leadboat.com> wrote in
> Diffing the two latest versions of one patch:
> > --- v32-0002-Fix-the-defect-1.patch 2020-01-18 14:32:47.499129940 -0800
> > +++ v33-0002-Fix-the-defect-1.patch 2020-01-26 16:23:52.846391035 -0800
> > +@@ -2978,8 +3054,8 @@ AssertPendingSyncs_RelationCache(void)
> > + LOCKTAG_RELATION)
> > + continue;
> > + relid = ObjectIdGetDatum(locallock->tag.lock.locktag_field2);
> > +- r = RelationIdGetRelation(relid);
> > +- if (r == NULL)
> > ++ r = RelationIdGetRelationCache(relid);
>
> The purpose of this loop is to create relcache entries for rels locked in the
> current transaction. (The "r == NULL" case happens for rels no longer visible
> in catalogs. It is harmless.) Since RelationIdGetRelationCache() never
> creates a relcache entry, calling it defeats that purpose.
> RelationIdGetRelation() is the right function to call.
I thought that the all required entry exist in the cache but actually
it's safer that recreate dropped caches. Does the following works?
r = RelationIdGetRelation(relid);
+ /* if not found, fetch a "dropped" entry if any */
+ if (r == NULL)
+ r = RelationIdGetRelationCache(relid);
if (r == NULL)
continue;
> On Tue, Jan 21, 2020 at 06:45:57PM +0900, Kyotaro Horiguchi wrote:
> > Three other fixes not mentined above are made. One is the useless
> > rd_firstRelfilenodeSubid in the condition to dicide whether to
> > preserve or not a relcache entry
>
> It was not useless. Test case:
>
> create table t (c int);
> begin;
> alter table t alter c type bigint; -- sets rd_firstRelfilenodeSubid
> savepoint q; drop table t; rollback to q; -- forgets rd_firstRelfilenodeSubid
> commit; -- assertion failure, after s/RelationIdGetRelationCache/RelationIdGetRelation/ discussed above
Mmm? I thought somehow that that relcache entry never be dropped and I
believe I considered that case, of course. But yes, you're right.
I'll post upated version.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
