Re: patch submission: truncate trailing nulls from heap rows to reduce the size of the null bitmap [Review]

From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Jameison Martin <jameisonb(at)yahoo(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Kevin Grittner <kgrittn(at)mail(dot)com>, robertmhaas(at)gmail(dot)com, josh(at)agliodbs(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch submission: truncate trailing nulls from heap rows to reduce the size of the null bitmap [Review]
Date: 2013-06-24 20:05:27
Message-ID: CA+U5nMJOsrVGC7oAPRoca0+tQiYzk1sb+QFXMix0WCEU9qGw7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 24 June 2013 15:49, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Kapila <amit(dot)kapila(at)huawei(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2013-06-24 20:17:46 Re: proposal 9.4 plpgsql: allows access to call stack from GET DIAGNOSTICS statement
Previous Message Cédric Villemain 2013-06-24 20:02:00 Re: Bugfix and new feature for PGXS