Re: Less than ideal error reporting in pg_stat_statements

Peter Geoghegan <pg@heroku.com>

From: Peter Geoghegan <pg@heroku.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jim Nasby <Jim.Nasby@bluetreble.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2015-10-02T22:57:45Z
Lists: pgsql-hackers
On Fri, Oct 2, 2015 at 2:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Geoghegan <pg@heroku.com> writes:
>> I think that SIZE_MAX should be replaced by MaxAllocHugeSize before
>> the patch is committed. That should be perfectly portable.
>
> Hmm ... only back to 9.4, but I guess that's far enough.

I just realized that the existing gc_fail handler label within
gc_qtexts() lacks something like this, too:

unlink(PGSS_TEXT_FILE);
(void) AllocateFile(PGSS_TEXT_FILE, PG_BINARY_W);
SpinLockAcquire(&s->mutex);
pgss->extent = 0;
SpinLockRelease(&s->mutex);

I think it should do this anyway, but it makes particular sense in
light of the proposed changes. All existing failure cases within
gc_qtexts() seem like a good reason to give up forever.

The spinlock acquisition above is actually necessary despite the
n_writers trick, because that's only used by qtext_store().

-- 
Peter Geoghegan


Commits

  1. Be more wary about 32-bit integer overflow in pg_stat_statements.