On 2014-05-14 13:32:43 -0400, Tom Lane wrote: > Andres Freund <and...@2ndquadrant.com> writes: > > On 2014-05-14 12:15:27 -0400, Tom Lane wrote: > >> And why does the header > >> comment for RelationGetIndexList make no mention of this new side-effect? > >> Somebody did a seriously poor job of adding this functionality to > >> relcache. > > > It's not like it's not documented: There's a comment about it in struct > > RelationData. I must have missed that rd_oidindex has a comment abou > > it's lifetime over RelationGetIndexList(). > > If rd_replidindex is being managed like rd_oidindex, then it should be > managed just like rd_oidindex, including getting reset in all the places > rd_oidindex is. This might be just a matter of cleanliness but I think > it's important for readability and debuggability.
Agreed. I am not against resetting it. I think I might not have been aware of rd_oidindex when writing that code... > I notice also that rd_keyattr and rd_idattr have been implemented with > bad copies of the logic for rd_indexattr. This is at least leading > to a permanent memory leak in CacheMemoryContext during every relcache > flush, and maybe worse things. The bugs for rd_keyattr appear to predate > your patch though. Hm. Yes, the bitmapsets should be freed. I guess I copied the logic for keyattr and didn't find any relevant places that touch it. rd_keyattr should go back to 9.3. > Working on a patch for this now. One thing I'm wondering about is > RelationSetIndexList. It's probably okay for it not to touch rd_keyattr > and rd_idattr, but I'm not too clear on what the use cases for those > attnum sets are. rd_keyattr is used to determine whether a heap_update() changed any keys that could be referenced by a foreign key. That's then used to determine which locklevel an update requires. rd_idattr does something similar. It decides whether the configured REPLICA IDENTITY key has changed so whether to log the old primary key for logical decoding or not. I can't see why either would need to care about forced index lists right now, but will do a scan of the sources to see if I am wrong. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
