InsertPgAttributeTuple() and attcacheoff

Started by Peter Eisentrautabout 8 years ago4 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t39204
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 28, 2026 at 05:26 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t39204_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t39204_1 && git checkout t39204_1

Patchset v1 (message #1) is on t39204_1

Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

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

Attachments:

t39204_1
0001-InsertPgAttributeTuple-to-set-attcacheoff.patchtext/plain; charset=UTF-8; name=0001-InsertPgAttributeTuple-to-set-attcacheoff.patch; x-mac-creator=0; x-mac-type=0Download+4-12
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: InsertPgAttributeTuple() and attcacheoff

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

#3Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#2)
Re: InsertPgAttributeTuple() and attcacheoff

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

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#3)
Re: InsertPgAttributeTuple() and attcacheoff

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