Thread: InsertPgAttributeTuple() and attcacheoff
It seems to me that it would make sense if InsertPgAttributeTuple() were to set attcacheoff to -1 instead of taking it from the caller. InsertPgAttributeTuple() is the interface between in-memory tuple descriptors and on-disk pg_attribute, so it makes sense to give it the job of resetting attcacheoff. This avoids having all the callers having to do so. There are also pending patches that have to work around this in seemingly unnecessary ways. (The comment about the "very grotty code" that I'm removing is from a time when AppendAttributeTuples() would deform a tuple, reset attcacheoff, then reform the tuple -- hence grotty. This is long obsolete, since the tuple is now formed later.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> It seems to me that it would make sense if InsertPgAttributeTuple() were
> to set attcacheoff to -1 instead of taking it from the caller.
Looked this over, no objections.
I wonder whether we should set that field to -1 when we *read*
pg_attribute rows from disk, and be less fussed about what gets written
out. The only real advantage is that this'd protect us from foolish
manual changes to pg_attribute.attcacheoff entries, but that doesn't
seem negligible.
regards, tom lane
On Tue, Aug 14, 2018 at 3:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> It seems to me that it would make sense if InsertPgAttributeTuple() were >> to set attcacheoff to -1 instead of taking it from the caller. > > Looked this over, no objections. > > I wonder whether we should set that field to -1 when we *read* > pg_attribute rows from disk, and be less fussed about what gets written > out. The only real advantage is that this'd protect us from foolish > manual changes to pg_attribute.attcacheoff entries, but that doesn't > seem negligible. I wouldn't object to forcibly writing in -1 when we read the data, but I don't think it's a good idea to let values other than -1 get written to the disk. User-visible random nonsense in system catalogs seems like too much of a foot-gun to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 14/08/2018 17:52, Robert Haas wrote: > On Tue, Aug 14, 2018 at 3:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >>> It seems to me that it would make sense if InsertPgAttributeTuple() were >>> to set attcacheoff to -1 instead of taking it from the caller. >> >> Looked this over, no objections. >> >> I wonder whether we should set that field to -1 when we *read* >> pg_attribute rows from disk, and be less fussed about what gets written >> out. The only real advantage is that this'd protect us from foolish >> manual changes to pg_attribute.attcacheoff entries, but that doesn't >> seem negligible. > > I wouldn't object to forcibly writing in -1 when we read the data, but > I don't think it's a good idea to let values other than -1 get written > to the disk. User-visible random nonsense in system catalogs seems > like too much of a foot-gun to me. I agree. Committed as presented then. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services