Thanks! At Sun, 26 Jan 2020 20:22:01 -0800, Noah Misch <n...@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