Tom Lane <t...@sss.pgh.pa.us> 于2023年5月11日周四 00:32写道：
> Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> writes: > > And, the /* do not unlock till end of xact */, it looks like it's been > > there from day 1. It may be indicating that the ref count fo the new > > relation created in heap_create_with_catalog() will be decremented at > > the end of xact, but I'm not sure what it means. > > Hmm, I think it's been copied-and-pasted from somewhere. It's quite > common for us to not release locks on modified tables until end of > transaction. However, that's not what's happening here, because we > actually *don't have any such lock* at this point, as you can easily > prove by stepping through this code and watching the contents of > pg_locks from another session. We do acquire AccessExclusiveLock > on the new table eventually, but not till control returns to > DefineRelation. > > I'm not real sure that I like the proposed code change: it's unclear > to me whether it's an unwise piercing of a couple of abstraction > layers or an okay change because those abstraction layers haven't > yet been applied to the new relation at all. However, I think the > existing comment is actively misleading and needs to be changed. > Maybe something like > > /* > * Close the relcache entry, since we return only an OID not a > * relcache reference. Note that we do not yet hold any lockmanager > * lock on the new rel, so there's nothing to release. > */ > table_close(new_rel_desc, NoLock); > > /* > * ok, the relation has been cataloged, so close catalogs and return > * the OID of the newly created relation. > */ > table_close(pg_class_desc, RowExclusiveLock); > +1 Personally, I prefer above code. Given these comments, maybe changing the first call to RelationClose > would be sensible, but I'm still not quite convinced. > > regards, tom lane > > >
