pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedtuplesort test coverage
10+ messages / 3 participants
[nested] [flat]
* tuplesort test coverage
@ 2019-10-13 14:41 Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 10+ messages in thread
From: Andres Freund @ 2019-10-13 14:41 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
Hi,
[1] made me look at tuplesorts test coverage at
https://coverage.postgresql.org/src/backend/utils/sort/tuplesort.c.gcov.html
We don't have coverage for a quite a number of things:
- cluster for expression indexes (line 935)
- sorts exceeding INT_MAX / 2 memory (line 1337), but that seems hard to
test realistically
- aborted abbreviated keys (lines 1522, 1608, 1774, 3620, 3739, 3867, 4266)
- in memory backwards scans (lines 1936, 3042)
- *any* coverage for TSS_SORTEDONTAPE (line 1964)
- disk sort skiptuples (line 2325)
- mergeruns without abbrev key (line 2582)
- disk sorts with more than one run (lines 2707, 2789)
- any disk based tuplesort_begin_heap() (lines 3649, 3676)
- Seems copytup_index currently is essentially dead, because
tuplesort_putindextuplevalues() doesn't use COPYTUP (line 4142)
- any disk based tuplesort_begin_datum (lines 4282, 4323)
I'm pretty unhappy that tuplesort has been whacked around pretty heavily
in the last few years, while *reducing* effective test coverage
noticeably, rather than increasing it. There's pretty substantial and
nontrivial areas without any tests - do we have actually have any
confidence that they work?
The largest culprits for that seem to be abbreviated keys, the tape
logic overhaul, and the increase of work mem.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-10-15 12:05 Peter Geoghegan <pg@bowt.ie>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 10+ messages in thread
From: Peter Geoghegan @ 2019-10-15 12:05 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
On Sun, Oct 13, 2019 at 3:41 PM Andres Freund <andres@anarazel.de> wrote:
> - cluster for expression indexes (line 935)
We've never had coverage of this, but perhaps that can be added now.
> - sorts exceeding INT_MAX / 2 memory (line 1337), but that seems hard to
> test realistically
I don't think that that can be tested, realistically.
> - 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.
> - in memory backwards scans (lines 1936, 3042)
> - *any* coverage for TSS_SORTEDONTAPE (line 1964)
That used to exist, but it went away when we killed replacement selection sort.
> - disk sort skiptuples (line 2325)
Couldn't hurt.
> - mergeruns without abbrev key (line 2582)
mergeruns() doesn't use abbreviated keys -- this code disables their
use in the standard way.
> - disk sorts with more than one run (lines 2707, 2789)
> - any disk based tuplesort_begin_heap() (lines 3649, 3676)
I had to convince Tom to get the coverage of external sorts we have
now. Apparently there are buildfarm animals that are very sensitive to
that cost, that could have substantially increased test runtimes were
we to do more. Perhaps this could be revisited.
> - Seems copytup_index currently is essentially dead, because
> tuplesort_putindextuplevalues() doesn't use COPYTUP (line 4142)
Yeah, that looks like dead code -- it should just be a stub with a
"can't happen" error.
> I'm pretty unhappy that tuplesort has been whacked around pretty heavily
> in the last few years, while *reducing* effective test coverage
> noticeably, rather than increasing it.
I don't think that that's true, on balance. There are only 1,000 extra
lines of code in tuplesort.c in master compared to 9.4, even though we
added parallel sorts and abbreviated keys, two huge enhancements. In
many ways, tuplesort is now simpler than ever.
> There's pretty substantial and
> nontrivial areas without any tests - do we have actually have any
> confidence that they work?
Everything that you're talking about has existed since v11 came out a
year ago, and most of it is a year or two older than that. So yeah,
I'm pretty confident that it works.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-10-24 18:10 Andres Freund <andres@anarazel.de>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 2 replies; 10+ messages in thread
From: Andres Freund @ 2019-10-24 18:10 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
Hi,
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.
> > - in memory backwards scans (lines 1936, 3042) - *any* coverage for
> > TSS_SORTEDONTAPE (line 1964)
>
> That used to exist, but it went away when we killed replacement selection sort.
Yes, that's kind of my point? Either that patch reduced coverage, or it
created dead code. Neither is good.
> > - mergeruns without abbrev key (line 2582)
>
> mergeruns() doesn't use abbreviated keys -- this code disables their
> use in the standard way.
Well, then reformulate the point that we should have coverage for
mergeruns() when initially abbreviated keys were set up.
> > - disk sorts with more than one run (lines 2707, 2789)
> > - any disk based tuplesort_begin_heap() (lines 3649, 3676)
>
> I had to convince Tom to get the coverage of external sorts we have
> now. Apparently there are buildfarm animals that are very sensitive to
> that cost, that could have substantially increased test runtimes were
> we to do more. Perhaps this could be revisited.
Hm. I'm a bit confused. Isn't all that's required to set a tiny amount
of work_mem? Then it's easy to trigger many passes without a lot of IO?
> > I'm pretty unhappy that tuplesort has been whacked around pretty heavily
> > in the last few years, while *reducing* effective test coverage
> > noticeably, rather than increasing it.
>
> I don't think that that's true, on balance. There are only 1,000 extra
> lines of code in tuplesort.c in master compared to 9.4, even though we
> added parallel sorts and abbreviated keys, two huge enhancements. In
> many ways, tuplesort is now simpler than ever.
I'm not saying that tuplesort has gotten worse or anything. Just that
there's been too much development without adding tests.
> > There's pretty substantial and
> > nontrivial areas without any tests - do we have actually have any
> > confidence that they work?
>
> Everything that you're talking about has existed since v11 came out a
> year ago, and most of it is a year or two older than that. So yeah,
> I'm pretty confident that it works.
That's may be true, but there's also basically no way to discover bugs
except manual testing, and users encountering the bugs. That's not good
enough.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-10-24 21:10 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 10+ messages in thread
From: Andres Freund @ 2019-10-24 21:10 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; 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
Attachments:
[text/x-diff] v1-0001-Add-tests-for-tuplesort.c.patch (44.0K, ../../20191024211027.owa65zwzuqevacbq@alap3.anarazel.de/2-v1-0001-Add-tests-for-tuplesort.c.patch)
download | inline diff:
From 3c987fa79d1d2341869de226879a7201e035040a Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Oct 2019 13:58:40 -0700
Subject: [PATCH v1 1/2] Add tests for tuplesort.c.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/test/regress/expected/tuplesort.out | 591 ++++++++++++++++++++++++
src/test/regress/parallel_schedule | 2 +-
src/test/regress/sql/tuplesort.sql | 258 +++++++++++
3 files changed, 850 insertions(+), 1 deletion(-)
create mode 100644 src/test/regress/expected/tuplesort.out
create mode 100644 src/test/regress/sql/tuplesort.sql
diff --git a/src/test/regress/expected/tuplesort.out b/src/test/regress/expected/tuplesort.out
new file mode 100644
index 00000000000..d3b99df73ae
--- /dev/null
+++ b/src/test/regress/expected/tuplesort.out
@@ -0,0 +1,591 @@
+-- only use parallelism when explicitly intending to do so
+SET max_parallel_maintenance_workers = 0;
+SET max_parallel_workers = 0;
+-- A table with with contents that, when sorted, triggers abbreviated
+-- key aborts. One easy way to achieve that is to use uuids that all
+-- have the same prefix, as abbreviated keys for uuids just use the
+-- first sizeof(Datum) bytes.
+DROP TABLE IF EXISTS abbrev_abort_uuids;
+NOTICE: table "abbrev_abort_uuids" does not exist, skipping
+CREATE TABLE abbrev_abort_uuids (
+ id serial not null,
+ abort_increasing uuid,
+ abort_decreasing uuid,
+ noabort_increasing uuid,
+ noabort_decreasing uuid);
+INSERT INTO abbrev_abort_uuids (abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing)
+ SELECT
+ ('00000000-0000-0000-0000-'||to_char(g.i, '000000000000FM'))::uuid abort_increasing,
+ ('00000000-0000-0000-0000-'||to_char(20000 - g.i, '000000000000FM'))::uuid abort_decreasing,
+ (to_char(g.i % 10009, '00000000FM')||'-0000-0000-0000-'||to_char(g.i, '000000000000FM'))::uuid noabort_increasing,
+ (to_char(((20000 - g.i) % 10009), '00000000FM')||'-0000-0000-0000-'||to_char(20000 - g.i, '000000000000FM'))::uuid noabort_decreasing
+ FROM generate_series(0, 20000, 1) g(i);
+-- and a few NULLs
+INSERT INTO abbrev_abort_uuids(id) VALUES(0);
+INSERT INTO abbrev_abort_uuids DEFAULT VALUES;
+INSERT INTO abbrev_abort_uuids DEFAULT VALUES;
+-- add just a few duplicates
+INSERT INTO abbrev_abort_uuids (abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing)
+ SELECT abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+ FROM abbrev_abort_uuids
+ WHERE (id < 10 OR id > 19990) AND id % 3 = 0 AND abort_increasing is not null;
+----
+-- Check sort node uses of tuplesort wrt. abbreviated keys
+----
+-- plain sort triggering abbreviated abort
+SELECT abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_increasing OFFSET 20000 - 4;
+ abort_increasing | abort_decreasing
+--------------------------------------+--------------------------------------
+ 00000000-0000-0000-0000-000000019992 | 00000000-0000-0000-0000-000000000008
+ 00000000-0000-0000-0000-000000019993 | 00000000-0000-0000-0000-000000000007
+ 00000000-0000-0000-0000-000000019994 | 00000000-0000-0000-0000-000000000006
+ 00000000-0000-0000-0000-000000019994 | 00000000-0000-0000-0000-000000000006
+ 00000000-0000-0000-0000-000000019995 | 00000000-0000-0000-0000-000000000005
+ 00000000-0000-0000-0000-000000019996 | 00000000-0000-0000-0000-000000000004
+ 00000000-0000-0000-0000-000000019997 | 00000000-0000-0000-0000-000000000003
+ 00000000-0000-0000-0000-000000019997 | 00000000-0000-0000-0000-000000000003
+ 00000000-0000-0000-0000-000000019998 | 00000000-0000-0000-0000-000000000002
+ 00000000-0000-0000-0000-000000019999 | 00000000-0000-0000-0000-000000000001
+ 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ |
+ |
+ |
+(15 rows)
+
+SELECT abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing NULLS FIRST OFFSET 20000 - 4;
+ abort_increasing | abort_decreasing
+--------------------------------------+--------------------------------------
+ 00000000-0000-0000-0000-000000000011 | 00000000-0000-0000-0000-000000019989
+ 00000000-0000-0000-0000-000000000010 | 00000000-0000-0000-0000-000000019990
+ 00000000-0000-0000-0000-000000000009 | 00000000-0000-0000-0000-000000019991
+ 00000000-0000-0000-0000-000000000008 | 00000000-0000-0000-0000-000000019992
+ 00000000-0000-0000-0000-000000000008 | 00000000-0000-0000-0000-000000019992
+ 00000000-0000-0000-0000-000000000007 | 00000000-0000-0000-0000-000000019993
+ 00000000-0000-0000-0000-000000000006 | 00000000-0000-0000-0000-000000019994
+ 00000000-0000-0000-0000-000000000005 | 00000000-0000-0000-0000-000000019995
+ 00000000-0000-0000-0000-000000000005 | 00000000-0000-0000-0000-000000019995
+ 00000000-0000-0000-0000-000000000004 | 00000000-0000-0000-0000-000000019996
+ 00000000-0000-0000-0000-000000000003 | 00000000-0000-0000-0000-000000019997
+ 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998
+ 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998
+ 00000000-0000-0000-0000-000000000001 | 00000000-0000-0000-0000-000000019999
+ 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000020000
+(15 rows)
+
+-- plain sort not triggering abbreviated abort
+SELECT noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_increasing OFFSET 20000 - 4;
+ noabort_increasing | noabort_decreasing
+--------------------------------------+--------------------------------------
+ 00009997-0000-0000-0000-000000009997 | 00010003-0000-0000-0000-000000010003
+ 00009998-0000-0000-0000-000000009998 | 00010002-0000-0000-0000-000000010002
+ 00009999-0000-0000-0000-000000009999 | 00010001-0000-0000-0000-000000010001
+ 00010000-0000-0000-0000-000000010000 | 00010000-0000-0000-0000-000000010000
+ 00010001-0000-0000-0000-000000010001 | 00009999-0000-0000-0000-000000009999
+ 00010002-0000-0000-0000-000000010002 | 00009998-0000-0000-0000-000000009998
+ 00010003-0000-0000-0000-000000010003 | 00009997-0000-0000-0000-000000009997
+ 00010004-0000-0000-0000-000000010004 | 00009996-0000-0000-0000-000000009996
+ 00010005-0000-0000-0000-000000010005 | 00009995-0000-0000-0000-000000009995
+ 00010006-0000-0000-0000-000000010006 | 00009994-0000-0000-0000-000000009994
+ 00010007-0000-0000-0000-000000010007 | 00009993-0000-0000-0000-000000009993
+ 00010008-0000-0000-0000-000000010008 | 00009992-0000-0000-0000-000000009992
+ |
+ |
+ |
+(15 rows)
+
+SELECT noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing NULLS FIRST OFFSET 20000 - 4;
+ noabort_increasing | noabort_decreasing
+--------------------------------------+--------------------------------------
+ 00010006-0000-0000-0000-000000010006 | 00009994-0000-0000-0000-000000009994
+ 00010005-0000-0000-0000-000000010005 | 00009995-0000-0000-0000-000000009995
+ 00010004-0000-0000-0000-000000010004 | 00009996-0000-0000-0000-000000009996
+ 00010003-0000-0000-0000-000000010003 | 00009997-0000-0000-0000-000000009997
+ 00010002-0000-0000-0000-000000010002 | 00009998-0000-0000-0000-000000009998
+ 00010001-0000-0000-0000-000000010001 | 00009999-0000-0000-0000-000000009999
+ 00010000-0000-0000-0000-000000010000 | 00010000-0000-0000-0000-000000010000
+ 00009999-0000-0000-0000-000000009999 | 00010001-0000-0000-0000-000000010001
+ 00009998-0000-0000-0000-000000009998 | 00010002-0000-0000-0000-000000010002
+ 00009997-0000-0000-0000-000000009997 | 00010003-0000-0000-0000-000000010003
+ 00009996-0000-0000-0000-000000009996 | 00010004-0000-0000-0000-000000010004
+ 00009995-0000-0000-0000-000000009995 | 00010005-0000-0000-0000-000000010005
+ 00009994-0000-0000-0000-000000009994 | 00010006-0000-0000-0000-000000010006
+ 00009993-0000-0000-0000-000000009993 | 00010007-0000-0000-0000-000000010007
+ 00009992-0000-0000-0000-000000009992 | 00010008-0000-0000-0000-000000010008
+(15 rows)
+
+-- bounded sort (disables abbreviated keys)
+SELECT abort_increasing, noabort_increasing FROM abbrev_abort_uuids ORDER BY abort_increasing LIMIT 5;
+ abort_increasing | noabort_increasing
+--------------------------------------+--------------------------------------
+ 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000000000
+ 00000000-0000-0000-0000-000000000001 | 00000001-0000-0000-0000-000000000001
+ 00000000-0000-0000-0000-000000000002 | 00000002-0000-0000-0000-000000000002
+ 00000000-0000-0000-0000-000000000002 | 00000002-0000-0000-0000-000000000002
+ 00000000-0000-0000-0000-000000000003 | 00000003-0000-0000-0000-000000000003
+(5 rows)
+
+SELECT abort_increasing, noabort_increasing FROM abbrev_abort_uuids ORDER BY noabort_increasing NULLS FIRST LIMIT 5;
+ abort_increasing | noabort_increasing
+--------------------------------------+--------------------------------------
+ |
+ |
+ |
+ 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000000000
+ 00000000-0000-0000-0000-000000010009 | 00000000-0000-0000-0000-000000010009
+(5 rows)
+
+----
+-- Check index creation uses of tuplesort wrt. abbreviated keys
+----
+-- index creation using abbreviated keys successfully
+CREATE INDEX abbrev_abort_uuids__noabort_increasing_idx ON abbrev_abort_uuids (noabort_increasing);
+CREATE INDEX abbrev_abort_uuids__noabort_decreasing_idx ON abbrev_abort_uuids (noabort_decreasing);
+-- verify
+EXPLAIN SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_increasing LIMIT 5;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------------------
+ Limit (cost=0.29..0.69 rows=5 width=36)
+ -> Index Scan using abbrev_abort_uuids__noabort_increasing_idx on abbrev_abort_uuids (cost=0.29..1608.45 rows=20011 width=36)
+(2 rows)
+
+SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_increasing LIMIT 5;
+ id | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------
+ 1 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000
+ 10010 | 00000000-0000-0000-0000-000000010009 | 00009991-0000-0000-0000-000000009991
+ 2 | 00000001-0000-0000-0000-000000000001 | 00009990-0000-0000-0000-000000019999
+ 10011 | 00000001-0000-0000-0000-000000010010 | 00009990-0000-0000-0000-000000009990
+ 3 | 00000002-0000-0000-0000-000000000002 | 00009989-0000-0000-0000-000000019998
+(5 rows)
+
+EXPLAIN SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing LIMIT 5;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------------------
+ Limit (cost=0.29..0.69 rows=5 width=36)
+ -> Index Scan using abbrev_abort_uuids__noabort_decreasing_idx on abbrev_abort_uuids (cost=0.29..1608.45 rows=20011 width=36)
+(2 rows)
+
+SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing LIMIT 5;
+ id | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------
+ 20001 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20010 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 9992 | 00009991-0000-0000-0000-000000009991 | 00000000-0000-0000-0000-000000010009
+ 20000 | 00009990-0000-0000-0000-000000019999 | 00000001-0000-0000-0000-000000000001
+ 9991 | 00009990-0000-0000-0000-000000009990 | 00000001-0000-0000-0000-000000010010
+(5 rows)
+
+-- index creation using abbreviated keys, hitting abort
+CREATE INDEX abbrev_abort_uuids__abort_increasing_idx ON abbrev_abort_uuids (abort_increasing);
+CREATE INDEX abbrev_abort_uuids__abort_decreasing_idx ON abbrev_abort_uuids (abort_decreasing);
+-- verify
+EXPLAIN SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_increasing LIMIT 5;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------
+ Limit (cost=0.29..0.69 rows=5 width=36)
+ -> Index Scan using abbrev_abort_uuids__abort_increasing_idx on abbrev_abort_uuids (cost=0.29..1608.45 rows=20011 width=36)
+(2 rows)
+
+SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_increasing LIMIT 5;
+ id | abort_increasing | abort_decreasing
+-------+--------------------------------------+--------------------------------------
+ 1 | 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000020000
+ 2 | 00000000-0000-0000-0000-000000000001 | 00000000-0000-0000-0000-000000019999
+ 3 | 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998
+ 20004 | 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998
+ 4 | 00000000-0000-0000-0000-000000000003 | 00000000-0000-0000-0000-000000019997
+(5 rows)
+
+EXPLAIN SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing LIMIT 5;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------
+ Limit (cost=0.29..0.69 rows=5 width=36)
+ -> Index Scan using abbrev_abort_uuids__abort_decreasing_idx on abbrev_abort_uuids (cost=0.29..1608.45 rows=20011 width=36)
+(2 rows)
+
+SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing LIMIT 5;
+ id | abort_increasing | abort_decreasing
+-------+--------------------------------------+--------------------------------------
+ 20001 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20010 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20000 | 00000000-0000-0000-0000-000000019999 | 00000000-0000-0000-0000-000000000001
+ 19999 | 00000000-0000-0000-0000-000000019998 | 00000000-0000-0000-0000-000000000002
+ 19998 | 00000000-0000-0000-0000-000000019997 | 00000000-0000-0000-0000-000000000003
+(5 rows)
+
+----
+-- Check CLUSTER uses of tuplesort wrt. abbreviated keys
+----
+-- when aborting, increasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__abort_increasing_idx;
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 1 | 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000
+ 2 | 00000000-0000-0000-0000-000000000001 | 00000000-0000-0000-0000-000000019999 | 00000001-0000-0000-0000-000000000001 | 00009990-0000-0000-0000-000000019999
+ 3 | 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998 | 00000002-0000-0000-0000-000000000002 | 00009989-0000-0000-0000-000000019998
+ 20004 | 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998 | 00000002-0000-0000-0000-000000000002 | 00009989-0000-0000-0000-000000019998
+ 4 | 00000000-0000-0000-0000-000000000003 | 00000000-0000-0000-0000-000000019997 | 00000003-0000-0000-0000-000000000003 | 00009988-0000-0000-0000-000000019997
+(5 rows)
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 0 | | | |
+ 20002 | | | |
+ 20003 | | | |
+ 20001 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20010 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+(5 rows)
+
+ROLLBACK;
+-- when aborting, decreasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__abort_decreasing_idx;
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 20010 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20001 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20000 | 00000000-0000-0000-0000-000000019999 | 00000000-0000-0000-0000-000000000001 | 00009990-0000-0000-0000-000000019999 | 00000001-0000-0000-0000-000000000001
+ 19999 | 00000000-0000-0000-0000-000000019998 | 00000000-0000-0000-0000-000000000002 | 00009989-0000-0000-0000-000000019998 | 00000002-0000-0000-0000-000000000002
+ 20009 | 00000000-0000-0000-0000-000000019997 | 00000000-0000-0000-0000-000000000003 | 00009988-0000-0000-0000-000000019997 | 00000003-0000-0000-0000-000000000003
+(5 rows)
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 0 | | | |
+ 20002 | | | |
+ 20003 | | | |
+ 1 | 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000
+ 2 | 00000000-0000-0000-0000-000000000001 | 00000000-0000-0000-0000-000000019999 | 00000001-0000-0000-0000-000000000001 | 00009990-0000-0000-0000-000000019999
+(5 rows)
+
+ROLLBACK;
+-- when not aborting, increasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__noabort_increasing_idx;
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 1 | 00000000-0000-0000-0000-000000000000 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000
+ 10010 | 00000000-0000-0000-0000-000000010009 | 00000000-0000-0000-0000-000000009991 | 00000000-0000-0000-0000-000000010009 | 00009991-0000-0000-0000-000000009991
+ 2 | 00000000-0000-0000-0000-000000000001 | 00000000-0000-0000-0000-000000019999 | 00000001-0000-0000-0000-000000000001 | 00009990-0000-0000-0000-000000019999
+ 10011 | 00000000-0000-0000-0000-000000010010 | 00000000-0000-0000-0000-000000009990 | 00000001-0000-0000-0000-000000010010 | 00009990-0000-0000-0000-000000009990
+ 20004 | 00000000-0000-0000-0000-000000000002 | 00000000-0000-0000-0000-000000019998 | 00000002-0000-0000-0000-000000000002 | 00009989-0000-0000-0000-000000019998
+(5 rows)
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 0 | | | |
+ 20002 | | | |
+ 20003 | | | |
+ 10009 | 00000000-0000-0000-0000-000000010008 | 00000000-0000-0000-0000-000000009992 | 00010008-0000-0000-0000-000000010008 | 00009992-0000-0000-0000-000000009992
+ 10008 | 00000000-0000-0000-0000-000000010007 | 00000000-0000-0000-0000-000000009993 | 00010007-0000-0000-0000-000000010007 | 00009993-0000-0000-0000-000000009993
+(5 rows)
+
+ROLLBACK;
+-- when no aborting, decreasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__noabort_decreasing_idx;
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 20010 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 20001 | 00000000-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000 | 00009991-0000-0000-0000-000000020000 | 00000000-0000-0000-0000-000000000000
+ 9992 | 00000000-0000-0000-0000-000000009991 | 00000000-0000-0000-0000-000000010009 | 00009991-0000-0000-0000-000000009991 | 00000000-0000-0000-0000-000000010009
+ 20000 | 00000000-0000-0000-0000-000000019999 | 00000000-0000-0000-0000-000000000001 | 00009990-0000-0000-0000-000000019999 | 00000001-0000-0000-0000-000000000001
+ 9991 | 00000000-0000-0000-0000-000000009990 | 00000000-0000-0000-0000-000000010010 | 00009990-0000-0000-0000-000000009990 | 00000001-0000-0000-0000-000000010010
+(5 rows)
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ id | abort_increasing | abort_decreasing | noabort_increasing | noabort_decreasing
+-------+--------------------------------------+--------------------------------------+--------------------------------------+--------------------------------------
+ 0 | | | |
+ 20003 | | | |
+ 20002 | | | |
+ 9993 | 00000000-0000-0000-0000-000000009992 | 00000000-0000-0000-0000-000000010008 | 00009992-0000-0000-0000-000000009992 | 00010008-0000-0000-0000-000000010008
+ 9994 | 00000000-0000-0000-0000-000000009993 | 00000000-0000-0000-0000-000000010007 | 00009993-0000-0000-0000-000000009993 | 00010007-0000-0000-0000-000000010007
+(5 rows)
+
+ROLLBACK;
+----
+-- test forward and backward scans for in-memory and disk based tuplesort
+----
+-- in-memory
+BEGIN;
+SET LOCAL enable_indexscan = false;
+-- unfortunately can't show analyze output confirming sort method,
+-- the memory used output wouldn't be stable
+EXPLAIN (COSTS OFF) DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+ QUERY PLAN
+--------------------------------------
+ Sort
+ Sort Key: noabort_decreasing
+ -> Seq Scan on abbrev_abort_uuids
+(3 rows)
+
+DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+-- first and second
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+-- scroll beyond beginning
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+-- scroll beyond end end
+FETCH LAST FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+COMMIT;
+-- disk based
+BEGIN;
+SET LOCAL enable_indexscan = false;
+SET LOCAL work_mem = '100kB';
+-- unfortunately can't show analyze output confirming sort method,
+-- the memory used output wouldn't be stable
+EXPLAIN (COSTS OFF) DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+ QUERY PLAN
+--------------------------------------
+ Sort
+ Sort Key: noabort_decreasing
+ -> Seq Scan on abbrev_abort_uuids
+(3 rows)
+
+DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+-- first and second
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+-- scroll beyond beginning
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+-- scroll beyond end end
+FETCH LAST FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+FETCH BACKWARD FROM c;
+ noabort_decreasing
+--------------------
+
+(1 row)
+
+FETCH NEXT FROM c;
+ noabort_decreasing
+--------------------
+(0 rows)
+
+COMMIT;
+----
+-- test tuplesort using both in-memory and disk sort
+---
+-- memory based
+SELECT
+ -- fixed-width by-value datum
+ (array_agg(id ORDER BY id DESC NULLS FIRST))[0:5],
+ -- fixed-width by-ref datum
+ (array_agg(abort_increasing ORDER BY abort_increasing DESC NULLS LAST))[0:5],
+ -- variable-width datum
+ (array_agg(id::text ORDER BY id::text DESC NULLS LAST))[0:5],
+ -- fixed width by-value datum tuplesort
+ percentile_disc(0.99) WITHIN GROUP (ORDER BY id),
+ -- ensure state is shared
+ percentile_disc(0.01) WITHIN GROUP (ORDER BY id),
+ -- fixed width by-ref datum tuplesort
+ percentile_disc(0.8) WITHIN GROUP (ORDER BY abort_increasing),
+ -- variable width by-ref datum tuplesort
+ percentile_disc(0.2) WITHIN GROUP (ORDER BY id::text),
+ -- multi-column tuplesort
+ rank('00000000-0000-0000-0000-000000000000', '2', '2') WITHIN GROUP (ORDER BY noabort_increasing, id, id::text)
+FROM (
+ SELECT * FROM abbrev_abort_uuids
+ UNION ALL
+ SELECT NULL, NULL, NULL, NULL, NULL) s;
+ array_agg | array_agg | array_agg | percentile_disc | percentile_disc | percentile_disc | percentile_disc | rank
+--------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+-----------------+-----------------+--------------------------------------+-----------------+------
+ {NULL,20010,20009,20008,20007} | {00000000-0000-0000-0000-000000020000,00000000-0000-0000-0000-000000020000,00000000-0000-0000-0000-000000019999,00000000-0000-0000-0000-000000019998,00000000-0000-0000-0000-000000019997} | {9999,9998,9997,9996,9995} | 19810 | 200 | 00000000-0000-0000-0000-000000016003 | 136 | 2
+(1 row)
+
+-- disk based (see also above)
+BEGIN;
+SET LOCAL work_mem = '100kB';
+SELECT
+ (array_agg(id ORDER BY id DESC NULLS FIRST))[0:5],
+ (array_agg(abort_increasing ORDER BY abort_increasing DESC NULLS LAST))[0:5],
+ (array_agg(id::text ORDER BY id::text DESC NULLS LAST))[0:5],
+ percentile_disc(0.99) WITHIN GROUP (ORDER BY id),
+ percentile_disc(0.01) WITHIN GROUP (ORDER BY id),
+ percentile_disc(0.8) WITHIN GROUP (ORDER BY abort_increasing),
+ percentile_disc(0.2) WITHIN GROUP (ORDER BY id::text),
+ rank('00000000-0000-0000-0000-000000000000', '2', '2') WITHIN GROUP (ORDER BY noabort_increasing, id, id::text)
+FROM (
+ SELECT * FROM abbrev_abort_uuids
+ UNION ALL
+ SELECT NULL, NULL, NULL, NULL, NULL) s;
+ array_agg | array_agg | array_agg | percentile_disc | percentile_disc | percentile_disc | percentile_disc | rank
+--------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+-----------------+-----------------+--------------------------------------+-----------------+------
+ {NULL,20010,20009,20008,20007} | {00000000-0000-0000-0000-000000020000,00000000-0000-0000-0000-000000020000,00000000-0000-0000-0000-000000019999,00000000-0000-0000-0000-000000019998,00000000-0000-0000-0000-000000019997} | {9999,9998,9997,9996,9995} | 19810 | 200 | 00000000-0000-0000-0000-000000016003 | 136 | 2
+(1 row)
+
+ROLLBACK;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index fc0f14122bb..d33a4e143dc 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -112,7 +112,7 @@ test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion tr
# ----------
# Another group of parallel tests
# ----------
-test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info
+test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info tuplesort
# event triggers cannot run concurrently with any test that runs DDL
test: event_trigger
diff --git a/src/test/regress/sql/tuplesort.sql b/src/test/regress/sql/tuplesort.sql
new file mode 100644
index 00000000000..ad47dd3168f
--- /dev/null
+++ b/src/test/regress/sql/tuplesort.sql
@@ -0,0 +1,258 @@
+-- only use parallelism when explicitly intending to do so
+SET max_parallel_maintenance_workers = 0;
+SET max_parallel_workers = 0;
+
+-- A table with with contents that, when sorted, triggers abbreviated
+-- key aborts. One easy way to achieve that is to use uuids that all
+-- have the same prefix, as abbreviated keys for uuids just use the
+-- first sizeof(Datum) bytes.
+DROP TABLE IF EXISTS abbrev_abort_uuids;
+
+CREATE TABLE abbrev_abort_uuids (
+ id serial not null,
+ abort_increasing uuid,
+ abort_decreasing uuid,
+ noabort_increasing uuid,
+ noabort_decreasing uuid);
+
+INSERT INTO abbrev_abort_uuids (abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing)
+ SELECT
+ ('00000000-0000-0000-0000-'||to_char(g.i, '000000000000FM'))::uuid abort_increasing,
+ ('00000000-0000-0000-0000-'||to_char(20000 - g.i, '000000000000FM'))::uuid abort_decreasing,
+ (to_char(g.i % 10009, '00000000FM')||'-0000-0000-0000-'||to_char(g.i, '000000000000FM'))::uuid noabort_increasing,
+ (to_char(((20000 - g.i) % 10009), '00000000FM')||'-0000-0000-0000-'||to_char(20000 - g.i, '000000000000FM'))::uuid noabort_decreasing
+ FROM generate_series(0, 20000, 1) g(i);
+
+-- and a few NULLs
+INSERT INTO abbrev_abort_uuids(id) VALUES(0);
+INSERT INTO abbrev_abort_uuids DEFAULT VALUES;
+INSERT INTO abbrev_abort_uuids DEFAULT VALUES;
+
+-- add just a few duplicates
+INSERT INTO abbrev_abort_uuids (abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing)
+ SELECT abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+ FROM abbrev_abort_uuids
+ WHERE (id < 10 OR id > 19990) AND id % 3 = 0 AND abort_increasing is not null;
+
+----
+-- Check sort node uses of tuplesort wrt. abbreviated keys
+----
+
+-- plain sort triggering abbreviated abort
+SELECT abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_increasing OFFSET 20000 - 4;
+SELECT abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing NULLS FIRST OFFSET 20000 - 4;
+
+-- plain sort not triggering abbreviated abort
+SELECT noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_increasing OFFSET 20000 - 4;
+SELECT noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing NULLS FIRST OFFSET 20000 - 4;
+
+-- bounded sort (disables abbreviated keys)
+SELECT abort_increasing, noabort_increasing FROM abbrev_abort_uuids ORDER BY abort_increasing LIMIT 5;
+SELECT abort_increasing, noabort_increasing FROM abbrev_abort_uuids ORDER BY noabort_increasing NULLS FIRST LIMIT 5;
+
+
+----
+-- Check index creation uses of tuplesort wrt. abbreviated keys
+----
+
+-- index creation using abbreviated keys successfully
+CREATE INDEX abbrev_abort_uuids__noabort_increasing_idx ON abbrev_abort_uuids (noabort_increasing);
+CREATE INDEX abbrev_abort_uuids__noabort_decreasing_idx ON abbrev_abort_uuids (noabort_decreasing);
+
+-- verify
+EXPLAIN SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_increasing LIMIT 5;
+SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_increasing LIMIT 5;
+EXPLAIN SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing LIMIT 5;
+SELECT id, noabort_increasing, noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing LIMIT 5;
+
+-- index creation using abbreviated keys, hitting abort
+CREATE INDEX abbrev_abort_uuids__abort_increasing_idx ON abbrev_abort_uuids (abort_increasing);
+CREATE INDEX abbrev_abort_uuids__abort_decreasing_idx ON abbrev_abort_uuids (abort_decreasing);
+
+-- verify
+EXPLAIN SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_increasing LIMIT 5;
+SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_increasing LIMIT 5;
+EXPLAIN SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing LIMIT 5;
+SELECT id, abort_increasing, abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing LIMIT 5;
+
+
+----
+-- Check CLUSTER uses of tuplesort wrt. abbreviated keys
+----
+
+-- when aborting, increasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__abort_increasing_idx;
+
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ROLLBACK;
+
+-- when aborting, decreasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__abort_decreasing_idx;
+
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ROLLBACK;
+
+-- when not aborting, increasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__noabort_increasing_idx;
+
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ROLLBACK;
+
+-- when no aborting, decreasing order
+BEGIN;
+SET LOCAL enable_indexscan = false;
+CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__noabort_decreasing_idx;
+
+-- head
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid LIMIT 5;
+
+-- tail
+SELECT id, abort_increasing, abort_decreasing, noabort_increasing, noabort_decreasing
+FROM abbrev_abort_uuids
+ORDER BY ctid DESC LIMIT 5;
+ROLLBACK;
+
+----
+-- test forward and backward scans for in-memory and disk based tuplesort
+----
+
+-- in-memory
+BEGIN;
+SET LOCAL enable_indexscan = false;
+-- unfortunately can't show analyze output confirming sort method,
+-- the memory used output wouldn't be stable
+EXPLAIN (COSTS OFF) DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+
+-- first and second
+FETCH NEXT FROM c;
+FETCH NEXT FROM c;
+
+-- scroll beyond beginning
+FETCH BACKWARD FROM c;
+FETCH BACKWARD FROM c;
+FETCH BACKWARD FROM c;
+FETCH BACKWARD FROM c;
+FETCH NEXT FROM c;
+
+-- scroll beyond end end
+FETCH LAST FROM c;
+FETCH BACKWARD FROM c;
+FETCH NEXT FROM c;
+FETCH NEXT FROM c;
+FETCH NEXT FROM c;
+FETCH BACKWARD FROM c;
+FETCH NEXT FROM c;
+
+COMMIT;
+
+-- disk based
+BEGIN;
+SET LOCAL enable_indexscan = false;
+SET LOCAL work_mem = '100kB';
+-- unfortunately can't show analyze output confirming sort method,
+-- the memory used output wouldn't be stable
+EXPLAIN (COSTS OFF) DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+DECLARE c SCROLL CURSOR FOR SELECT noabort_decreasing FROM abbrev_abort_uuids ORDER BY noabort_decreasing;
+
+-- first and second
+FETCH NEXT FROM c;
+FETCH NEXT FROM c;
+
+-- scroll beyond beginning
+FETCH BACKWARD FROM c;
+FETCH BACKWARD FROM c;
+FETCH BACKWARD FROM c;
+FETCH BACKWARD FROM c;
+FETCH NEXT FROM c;
+
+-- scroll beyond end end
+FETCH LAST FROM c;
+FETCH BACKWARD FROM c;
+FETCH NEXT FROM c;
+FETCH NEXT FROM c;
+FETCH NEXT FROM c;
+FETCH BACKWARD FROM c;
+FETCH NEXT FROM c;
+
+COMMIT;
+
+
+----
+-- test tuplesort using both in-memory and disk sort
+---
+
+-- memory based
+SELECT
+ -- fixed-width by-value datum
+ (array_agg(id ORDER BY id DESC NULLS FIRST))[0:5],
+ -- fixed-width by-ref datum
+ (array_agg(abort_increasing ORDER BY abort_increasing DESC NULLS LAST))[0:5],
+ -- variable-width datum
+ (array_agg(id::text ORDER BY id::text DESC NULLS LAST))[0:5],
+ -- fixed width by-value datum tuplesort
+ percentile_disc(0.99) WITHIN GROUP (ORDER BY id),
+ -- ensure state is shared
+ percentile_disc(0.01) WITHIN GROUP (ORDER BY id),
+ -- fixed width by-ref datum tuplesort
+ percentile_disc(0.8) WITHIN GROUP (ORDER BY abort_increasing),
+ -- variable width by-ref datum tuplesort
+ percentile_disc(0.2) WITHIN GROUP (ORDER BY id::text),
+ -- multi-column tuplesort
+ rank('00000000-0000-0000-0000-000000000000', '2', '2') WITHIN GROUP (ORDER BY noabort_increasing, id, id::text)
+FROM (
+ SELECT * FROM abbrev_abort_uuids
+ UNION ALL
+ SELECT NULL, NULL, NULL, NULL, NULL) s;
+
+-- disk based (see also above)
+BEGIN;
+SET LOCAL work_mem = '100kB';
+
+SELECT
+ (array_agg(id ORDER BY id DESC NULLS FIRST))[0:5],
+ (array_agg(abort_increasing ORDER BY abort_increasing DESC NULLS LAST))[0:5],
+ (array_agg(id::text ORDER BY id::text DESC NULLS LAST))[0:5],
+ percentile_disc(0.99) WITHIN GROUP (ORDER BY id),
+ percentile_disc(0.01) WITHIN GROUP (ORDER BY id),
+ percentile_disc(0.8) WITHIN GROUP (ORDER BY abort_increasing),
+ percentile_disc(0.2) WITHIN GROUP (ORDER BY id::text),
+ rank('00000000-0000-0000-0000-000000000000', '2', '2') WITHIN GROUP (ORDER BY noabort_increasing, id, id::text)
+FROM (
+ SELECT * FROM abbrev_abort_uuids
+ UNION ALL
+ SELECT NULL, NULL, NULL, NULL, NULL) s;
+
+ROLLBACK;
--
2.23.0.385.gbc12974a89
[text/x-diff] v1-0002-Remove-unused-code-from-tuplesort.patch (2.8K, ../../20191024211027.owa65zwzuqevacbq@alap3.anarazel.de/3-v1-0002-Remove-unused-code-from-tuplesort.patch)
download | inline diff:
From c4d4548149d87e8b29ad9f958a5776e09a9f9077 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 24 Oct 2019 13:58:53 -0700
Subject: [PATCH v1 2/2] Remove unused code from tuplesort.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/backend/utils/sort/tuplesort.c | 63 +-----------------------------
1 file changed, 2 insertions(+), 61 deletions(-)
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index a507bd24984..7947d2bca0a 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -4141,67 +4141,8 @@ comparetup_index_hash(const SortTuple *a, const SortTuple *b,
static void
copytup_index(Tuplesortstate *state, SortTuple *stup, void *tup)
{
- IndexTuple tuple = (IndexTuple) tup;
- unsigned int tuplen = IndexTupleSize(tuple);
- IndexTuple newtuple;
- Datum original;
-
- /* copy the tuple into sort storage */
- newtuple = (IndexTuple) MemoryContextAlloc(state->tuplecontext, tuplen);
- memcpy(newtuple, tuple, tuplen);
- USEMEM(state, GetMemoryChunkSpace(newtuple));
- stup->tuple = (void *) newtuple;
- /* set up first-column key value */
- original = index_getattr(newtuple,
- 1,
- RelationGetDescr(state->indexRel),
- &stup->isnull1);
-
- if (!state->sortKeys->abbrev_converter || stup->isnull1)
- {
- /*
- * Store ordinary Datum representation, or NULL value. If there is a
- * converter it won't expect NULL values, and cost model is not
- * required to account for NULL, so in that case we avoid calling
- * converter and just set datum1 to zeroed representation (to be
- * consistent, and to support cheap inequality tests for NULL
- * abbreviated keys).
- */
- stup->datum1 = original;
- }
- else if (!consider_abort_common(state))
- {
- /* Store abbreviated key representation */
- stup->datum1 = state->sortKeys->abbrev_converter(original,
- state->sortKeys);
- }
- else
- {
- /* Abort abbreviation */
- int i;
-
- stup->datum1 = original;
-
- /*
- * Set state to be consistent with never trying abbreviation.
- *
- * Alter datum1 representation in already-copied tuples, so as to
- * ensure a consistent representation (current tuple was just
- * handled). It does not matter if some dumped tuples are already
- * sorted on tape, since serialized tuples lack abbreviated keys
- * (TSS_BUILDRUNS state prevents control reaching here in any case).
- */
- for (i = 0; i < state->memtupcount; i++)
- {
- SortTuple *mtup = &state->memtuples[i];
-
- tuple = (IndexTuple) mtup->tuple;
- mtup->datum1 = index_getattr(tuple,
- 1,
- RelationGetDescr(state->indexRel),
- &mtup->isnull1);
- }
- }
+ /* Not currently needed */
+ elog(ERROR, "copytup_index() should not be called");
}
static void
--
2.23.0.385.gbc12974a89
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-10-25 11:30 Peter Geoghegan <pg@bowt.ie>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 10+ messages in thread
From: Peter Geoghegan @ 2019-10-25 11:30 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
On Thu, Oct 24, 2019 at 7:10 PM Andres Freund <andres@anarazel.de> wrote:
> 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.
I agree.
> 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.
I would like to improve this. I am mostly just pointing out that there
has been resistance to this historically. I am in favor of
mechanically increasing test coverage of tuplesort.c along the lines
you describe. I'm just a bit concerned that Tom or others may see it
differently.
> 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.
I agree.
> Yes, that's kind of my point? Either that patch reduced coverage, or it
> created dead code. Neither is good.
I agree.
> > mergeruns() doesn't use abbreviated keys -- this code disables their
> > use in the standard way.
>
> Well, then reformulate the point that we should have coverage for
> mergeruns() when initially abbreviated keys were set up.
That doesn't seem essentially, but I'm okay with it.
> > I had to convince Tom to get the coverage of external sorts we have
> > now. Apparently there are buildfarm animals that are very sensitive to
> > that cost, that could have substantially increased test runtimes were
> > we to do more. Perhaps this could be revisited.
>
> Hm. I'm a bit confused. Isn't all that's required to set a tiny amount
> of work_mem? Then it's easy to trigger many passes without a lot of IO?
Yes, but Tom felt that this might not be good enough when this was
discussed in 2016. However, I seem to recall that he was pleasantly
surprised at how small the overhead turned out to be.
It's hard for me to test how much overhead this will have on a machine
with horribly slow I/O. Though I just bought a new Raspberry Pi, and
could test on that when I get back home from my trip to Europe -- it
uses an SD card, which is pretty slow.
> I'm not saying that tuplesort has gotten worse or anything. Just that
> there's been too much development without adding tests.
I agree.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-10-25 11:37 Peter Geoghegan <pg@bowt.ie>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 10+ messages in thread
From: Peter Geoghegan @ 2019-10-25 11:37 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
On Thu, Oct 24, 2019 at 10:10 PM Andres Freund <andres@anarazel.de> wrote:
> 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.
Yeah. It looks like function coverage of logtape.c will be 100% once
you have coverage of mark and restore.
> 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.
That's hard for me to test right now, but offhand this general
approach looks good to me. I am pretty sure it's portable.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-11-14 00:25 Andres Freund <andres@anarazel.de>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 10+ messages in thread
From: Andres Freund @ 2019-11-14 00:25 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
Hi,
On 2019-10-25 12:37:38 +0100, Peter Geoghegan wrote:
> On Thu, Oct 24, 2019 at 10:10 PM Andres Freund <andres@anarazel.de> wrote:
> > 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.
>
> Yeah. It looks like function coverage of logtape.c will be 100% once
> you have coverage of mark and restore.
Yea, it's definitely better after.
> > 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.
>
> That's hard for me to test right now, but offhand this general
> approach looks good to me. I am pretty sure it's portable.
I pushed this now. We'll see what the slower buildfarm animals say. I'll
try to see how long they took in a few days.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-12-12 14:27 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2019-12-12 14:27 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Peter Geoghegan <pg@bowt.ie>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> I pushed this now. We'll see what the slower buildfarm animals say. I'll
> try to see how long they took in a few days.
friarbird (a CLOBBER_CACHE_ALWAYS animal) just showed a failure in this:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=friarbird&dt=2019-12-12%2006%3A20%3A02
================== pgsql.build/src/test/regress/regression.diffs ===================
diff -U3 /pgbuild/root/HEAD/pgsql.build/../pgsql/src/test/regress/expected/tuplesort.out /pgbuild/root/HEAD/pgsql.build/src/test/regress/results/tuplesort.out
--- /pgbuild/root/HEAD/pgsql.build/../pgsql/src/test/regress/expected/tuplesort.out 2019-11-13 19:54:11.000000000 -0500
+++ /pgbuild/root/HEAD/pgsql.build/src/test/regress/results/tuplesort.out 2019-12-12 08:25:23.000000000 -0500
@@ -625,13 +625,13 @@
Group Key: a.col12
Filter: (count(*) > 1)
-> Merge Join
- Merge Cond: (a.col12 = b.col12)
- -> Sort
- Sort Key: a.col12 DESC
- -> Seq Scan on test_mark_restore a
+ Merge Cond: (b.col12 = a.col12)
-> Sort
Sort Key: b.col12 DESC
-> Seq Scan on test_mark_restore b
+ -> Sort
+ Sort Key: a.col12 DESC
+ -> Seq Scan on test_mark_restore a
(14 rows)
:qry;
Since a and b are exactly the same table, in principle it's a matter of
chance which one the planner will put on the outside of the join.
I think what happened here is that the test ran long enough for
autovacuum/autoanalyze to come along and scan the table, changing its
stats in between where the planner picked up the stats for a and those
for b, and we ended up making the opposite join order choice.
I considered fixing this by adding some restriction clause on b so
that the join order choice isn't such a coin flip. But it's not
clear that the problem couldn't recur anyway --- the table stats
would change significantly on auto-analyze, since the test script
isn't bothering to create any stats itself.
What seems like a simpler and more reliable fix is to make
test_mark_restore a temp table, thus keeping autovac away from it.
Is there a reason in terms of the test's goals not to do that?
Also ... why in the world does the script drop its tables at the end
with IF EXISTS? They'd better exist at that point. I object
to the DROP IF EXISTS up at the top, too. The regression tests
do not need to be designed to deal with an unpredictable start state,
and coding them to do so can have no effect other than possibly
masking problems.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-12-12 23:25 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 10+ messages in thread
From: Andres Freund @ 2019-12-12 23:25 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Geoghegan <pg@bowt.ie>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
Hi,
On 2019-12-12 09:27:04 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I pushed this now. We'll see what the slower buildfarm animals say. I'll
> > try to see how long they took in a few days.
>
> friarbird (a CLOBBER_CACHE_ALWAYS animal) just showed a failure in this:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=friarbird&dt=2019-12-12%2006%3A20%3A02
>
> ================== pgsql.build/src/test/regress/regression.diffs ===================
> diff -U3 /pgbuild/root/HEAD/pgsql.build/../pgsql/src/test/regress/expected/tuplesort.out /pgbuild/root/HEAD/pgsql.build/src/test/regress/results/tuplesort.out
> --- /pgbuild/root/HEAD/pgsql.build/../pgsql/src/test/regress/expected/tuplesort.out 2019-11-13 19:54:11.000000000 -0500
> +++ /pgbuild/root/HEAD/pgsql.build/src/test/regress/results/tuplesort.out 2019-12-12 08:25:23.000000000 -0500
> @@ -625,13 +625,13 @@
> Group Key: a.col12
> Filter: (count(*) > 1)
> -> Merge Join
> - Merge Cond: (a.col12 = b.col12)
> - -> Sort
> - Sort Key: a.col12 DESC
> - -> Seq Scan on test_mark_restore a
> + Merge Cond: (b.col12 = a.col12)
> -> Sort
> Sort Key: b.col12 DESC
> -> Seq Scan on test_mark_restore b
> + -> Sort
> + Sort Key: a.col12 DESC
> + -> Seq Scan on test_mark_restore a
> (14 rows)
>
> :qry;
>
> Since a and b are exactly the same table, in principle it's a matter of
> chance which one the planner will put on the outside of the join.
Yea.
> I think what happened here is that the test ran long enough for
> autovacuum/autoanalyze to come along and scan the table, changing its
> stats in between where the planner picked up the stats for a and those
> for b, and we ended up making the opposite join order choice.
Sounds reasonable.
> What seems like a simpler and more reliable fix is to make
> test_mark_restore a temp table, thus keeping autovac away from it.
> Is there a reason in terms of the test's goals not to do that?
I can't see any reason. The sorting code shouldn't care about the source
of tuples. I guess there could at some point be tests for parallel
sorting, but that'd just use a different table.
> Also ... why in the world does the script drop its tables at the end
> with IF EXISTS? They'd better exist at that point. I object
> to the DROP IF EXISTS up at the top, too. The regression tests
> do not need to be designed to deal with an unpredictable start state,
> and coding them to do so can have no effect other than possibly
> masking problems.
Well, it makes it a heck of a lot easier to run tests in isolation while
evolving them. While I personally think it's good to leave cleanup for
partial states in for cases where it was helpful during development, I
also don't care about it strongly.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: tuplesort test coverage
@ 2019-12-14 20:03 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 10+ messages in thread
From: Tom Lane @ 2019-12-14 20:03 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Peter Geoghegan <pg@bowt.ie>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; Robert Haas <robertmhaas@gmail.com>; pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2019-12-12 09:27:04 -0500, Tom Lane wrote:
>> What seems like a simpler and more reliable fix is to make
>> test_mark_restore a temp table, thus keeping autovac away from it.
>> Is there a reason in terms of the test's goals not to do that?
> I can't see any reason. The sorting code shouldn't care about the source
> of tuples. I guess there could at some point be tests for parallel
> sorting, but that'd just use a different table.
OK, done that way.
>> Also ... why in the world does the script drop its tables at the end
>> with IF EXISTS? They'd better exist at that point. I object
>> to the DROP IF EXISTS up at the top, too. The regression tests
>> do not need to be designed to deal with an unpredictable start state,
>> and coding them to do so can have no effect other than possibly
>> masking problems.
> Well, it makes it a heck of a lot easier to run tests in isolation while
> evolving them. While I personally think it's good to leave cleanup for
> partial states in for cases where it was helpful during development, I
> also don't care about it strongly.
As far as that goes, making the tables temp is an even better solution.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2019-12-14 20:03 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-10-13 14:41 tuplesort test coverage Andres Freund <andres@anarazel.de>
2019-10-15 12:05 ` Peter Geoghegan <pg@bowt.ie>
2019-10-24 18:10 ` Andres Freund <andres@anarazel.de>
2019-10-24 21:10 ` Andres Freund <andres@anarazel.de>
2019-10-25 11:37 ` Peter Geoghegan <pg@bowt.ie>
2019-11-14 00:25 ` Andres Freund <andres@anarazel.de>
2019-12-12 14:27 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-12-12 23:25 ` Andres Freund <andres@anarazel.de>
2019-12-14 20:03 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-10-25 11:30 ` Peter Geoghegan <pg@bowt.ie>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox