From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jeff Davis <jdavis(at)postgresql(dot)org> |
Cc: | pgsql-committers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pgsql: Logical Tape Set: lazily allocate read buffer. |
Date: | 2020-02-18 21:25:52 |
Message-ID: | 96262b6442f8f752f42b50e9cc8ee69e0d9b43fc.camel@j-davis.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
On Sun, 2020-02-16 at 10:51 -0500, Tom Lane wrote:
> Is there a reason for that to be coded in such an obscure and fragile
> fashion, rather than having the test be say "if (lt->buffer ==
> NULL)"?
I did that to match the original behavior, which is to only allocate
the read buffer if the tape is non-empty.
A tape with a NULL buffer is in a valid state after a rewind, though
it's precarious. It raises quite a few questions about what the valid
states of a tape are, and what usages of the API are allowed. That was
all true even before 7fdd919a (or perhaps I made a mistake and moving
the code around was not safe after all).
I think the best fix now is to just allocate the buffer even if the
tape is empty. That would stop Coverity from complaining, and I
couldn't detect any obvious performance regression.
Later, we can document the valid states a little better, and validate
them with Asserts and/or the type system.
Regards,
Jeff Davis
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2020-02-19 00:08:18 | pgsql: Remove obsolete _bt_compare() comment. |
Previous Message | Fujii Masao | 2020-02-18 04:15:30 | pgsql: Make inherited LOCK TABLE perform access permission checks on pa |