Re: patch submission: truncate trailing nulls from heap rows to reduce the size of the null bitmap [Review]
Simon Riggs <simon@2ndquadrant.com>
From: Simon Riggs <simon@2ndQuadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Kapila <amit.kapila@huawei.com>,
Andres Freund <andres@2ndquadrant.com>,
Craig Ringer <craig@2ndquadrant.com>,
Jameison Martin <jameisonb@yahoo.com>, Noah Misch <noah@leadboat.com>,
Kevin Grittner <kgrittn@mail.com>, robertmhaas@gmail.com,
josh@agliodbs.com, pgsql-hackers@postgresql.org
Date: 2013-06-24T20:05:27Z
Lists: pgsql-hackers
On 24 June 2013 15:49, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Amit Kapila <amit.kapila@huawei.com> writes: >> I will summarize the results, and if most of us feel that they are not good >> enough, then we can return this patch. > > Aside from the question of whether there's really any generally-useful > performance improvement from this patch, it strikes me that this change > forecloses other games that we might want to play with interpretation of > the value of a tuple's natts. > > In particular, when I was visiting Salesforce last week, the point came > up that they'd really like ALTER TABLE ADD COLUMN to be "free" even when > the column had a non-null DEFAULT. It's not too difficult to imagine > how we might support that, at least when the default expression is a > constant: decree that if the requested attribute number is beyond natts, > look aside at the tuple descriptor to see if the column is marked as > having a magic default value, and if so, substitute that rather than > just returning NULL. (This has to be a "magic" value separate from > the current default, else a subsequent DROP or SET DEFAULT would do > the wrong thing.) Now that is a mighty fine idea. > However, this idea conflicts with an optimization that supposes it can > reduce natts to suppress null columns: if the column was actually stored > as null, you'd lose that information, and would incorrectly return the > magic default on subsequent reads. > > I think it might be possible to make both ideas play together, by > not reducing natts further than the last column with a magic default. > However, that means extra complexity in heap_form_tuple, which would > invalidate the performance measurements done in support of this patch. > > In short, there's no free lunch ... Agreed. I think its rather a shame that the proponents of this patch have tried so hard to push this through without working variations on the theme. Please guys, go away and get creative; rethink the approach so that you can have your lunch without anybody else losing theirs. I would add that I have seen the use case and want to support it, but we're looking to add to the codebase not just steal small bites of performance from existing use cases. As a practical suggestion, why not change the macro so it doesn't even try to do anything different unless the number of columns is > 72. A constant comparison should go very quickly and isolate the code better from the more typical code path. If not, lets try some other ideas, like Tom just did. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Commits
-
Don't assume that a tuple's header size is unchanged during toasting.
- 039680affb1b 9.2.0 cited