Re: tuplesort test coverage

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, 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-12-12 23:25:21
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2019-12-12 09:27:04 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)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:
> ================== ===================
> diff -U3 /pgbuild/root/HEAD/ /pgbuild/root/HEAD/
> --- /pgbuild/root/HEAD/ 2019-11-13 19:54:11.000000000 -0500
> +++ /pgbuild/root/HEAD/ 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.

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.


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-12-12 23:25:26 Re: Corruption with duplicate primary key
Previous Message Alvaro Herrera 2019-12-12 23:19:01 Re: log bind parameter values on error