On Fri, May 19, 2023 at 8:05 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > I noticed that BF animal conchuela has several times fallen over on the > test case added by 558c9d75f: > > diff -U3 > /home/pgbf/buildroot/HEAD/pgsql.build/src/test/regress/expected/join_hash.out > /home/pgbf/buildroot/HEAD/pgsql.build/src/test/regress/results/join_hash.out > --- > /home/pgbf/buildroot/HEAD/pgsql.build/src/test/regress/expected/join_hash.out > 2023-04-19 10:20:26.159840000 +0200 > +++ > /home/pgbf/buildroot/HEAD/pgsql.build/src/test/regress/results/join_hash.out > 2023-04-19 10:21:47.971900000 +0200 > @@ -974,8 +974,8 @@ > SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON > t1.id = t2.id; > id | id > ----+---- > - 1 | > | 2 > + 1 | > (2 rows) > > -- Test serial full hash join. > > Considering that this is a parallel plan, I don't think there's any > mystery about why an ORDER-BY-less query might have unstable output > order; the only mystery is why more of the buildfarm hasn't failed. > Can we just add "ORDER BY t1.id" to this query? It looks like you > get the same PHJ plan, although now underneath Sort/Gather Merge.
Yes, this was an oversight on my part. Attached is the patch that does just what you suggested. I can't help but take this opportunity to bump my un-reviewed patch further upthread which adds additional test coverage for match bit clearing for multi-batch hash joins [1]. It happens to also remove the test that failed on the buildfarm, which is why I thought to bring it up. -- Melanie [1] https://www.postgresql.org/message-id/CAAKRu_bdwDN_aHVctHcc9VoDP9av7LUMeuLbch1fHD2ESouw1g%40mail.gmail.com
From d7b357c0f2e75bdfee1a58f98fae09702d8533d2 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <melanieplage...@gmail.com> Date: Wed, 7 Jun 2023 16:42:44 -0400 Subject: [PATCH v1] Fix hash full join test case with ORDER BY 558c9d7 added test coverage for hash join match bit initialization but neglected to include an ORDER BY in the parallel hash join test query leading to sporadic buildfarm failures. Reported by Tom Lane Discussion: https://postgr.es/m/623596.1684541098%40sss.pgh.pa.us --- src/test/regress/expected/join_hash.out | 3 ++- src/test/regress/sql/join_hash.sql | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/join_hash.out b/src/test/regress/expected/join_hash.out index e892e7cccb..4faf010f8c 100644 --- a/src/test/regress/expected/join_hash.out +++ b/src/test/regress/expected/join_hash.out @@ -971,7 +971,8 @@ INSERT INTO hjtest_matchbits_t2 VALUES (2); -- Update should create a HOT tuple. If this status bit isn't cleared, we won't -- correctly emit the NULL-extended unmatching tuple in full hash join. UPDATE hjtest_matchbits_t2 set id = 2; -SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id; +SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id + ORDER BY t1.id; id | id ----+---- 1 | diff --git a/src/test/regress/sql/join_hash.sql b/src/test/regress/sql/join_hash.sql index 06bab7a4c7..e73f645e9e 100644 --- a/src/test/regress/sql/join_hash.sql +++ b/src/test/regress/sql/join_hash.sql @@ -523,7 +523,8 @@ INSERT INTO hjtest_matchbits_t2 VALUES (2); -- Update should create a HOT tuple. If this status bit isn't cleared, we won't -- correctly emit the NULL-extended unmatching tuple in full hash join. UPDATE hjtest_matchbits_t2 set id = 2; -SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id; +SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id + ORDER BY t1.id; -- Test serial full hash join. -- Resetting parallel_setup_cost should force a serial plan. -- Just to be safe, however, set enable_parallel_hash to off, as parallel full -- 2.37.2
