From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Peter Geoghegan <pg(at)bowt(dot)ie>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: tuplesort test coverage |
Date: | 2019-10-24 21:10:27 |
Message-ID: | 20191024211027.owa65zwzuqevacbq@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2019-10-24 11:10:34 -0700, Andres Freund wrote:
> On 2019-10-15 13:05:32 +0100, Peter Geoghegan wrote:
> > > - aborted abbreviated keys (lines 1522, 1608, 1774, 3620, 3739, 3867, 4266)
> >
> > Also hard to test -- there was a bug here when abbreviated keys first
> > went in -- that was detected by amcheck.
> >
> > All of the places where we abort are essentially the same, though.
>
> Why is it that hard? Seems fairly easy to create cases that reliably
> abort.
>
> I really don't think it's ok to have as many abbrev abort related paths
> without any coverage - the relevant code isn't that trivial. And
> something like amcheck really doesn't strike me as sufficient. For one,
> it doesn't provide any coverage either. For another, plenty sorts don't
> end up in a form that amcheck sees.
>
> Tests aren't just there to verify that the current behaviour isn't
> broken, they're also there to allow to develop with some confidence. And
> I don't think tuplesort as is really allows that, and e.g. abbreviated
> keys made that substantially worse. That happens, but I think it'd be
> good if you could help improving the situation.
>
> E.g.
> SELECT * FROM (SELECT ('00000000-0000-0000-0000-'||to_char(g.i, '000000000000FM'))::uuid uuid FROM generate_series(15000, 0, -1) g(i)) d ORDER BY uuid
> reliably triggers abbreviated keys, and it looks to me like that should
> be portable. With a few tweaks it'd be fairly easy to use that to
> provide some OK coverage for most the abbrev key cases.
Here's a first stab at getting the coverage of tuplesort.c to a
satisfying level. There's still bits uncovered, but that's largely
either a) trace_sort related b) hopefully unreachable stuff c) explain
related. The largest actually missing thing is a disk-based
mark/restore, which probably ought be covered.
I think the the test time of this would still be OK, but if not we could
also work a bit more on that angle.
I'm pretty sure there's some minor copy & paste mistakes in the test,
but I want to get this out there and get some reactions before investing
further time.
Peter, Tom?
- Andres
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Add-tests-for-tuplesort.c.patch | text/x-diff | 44.0 KB |
v1-0002-Remove-unused-code-from-tuplesort.patch | text/x-diff | 2.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2019-10-24 21:25:12 | Re: EXPLAIN BUFFERS and I/O timing accounting questions |
Previous Message | Alex Adriaanse | 2019-10-24 20:20:00 | TOAST corruption in standby database |