On Mon, Mar 15, 2021 at 6:14 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
> On Mon, Mar 15, 2021 at 9:54 AM Thomas Munro <thomas.mu...@gmail.com> wrote:
> > While reviewing the patch for parallel REFRESH MATERIALIZED VIEW, I
> > noticed that select_parallel.sql and write_parallel.sql believe that
> > (1) the tests are supposed to work with serializable as a default
> > isolation level, and (2) parallelism would be inhibited by that, so
> > they'd better use something else explicitly.  Here's a patch to update
> > that second thing in light of commit bb16aba5.  I don't think it
> > matters enough to bother back-patching it.
>
> +1, patch basically LGTM. I have one point - do we also need to remove
> "begin isolation level repeatable read;" in aggreates.sql, explain.sql
> and insert_parallel.sql? And in insert_parallel.sql, the comment also
> says "Serializable isolation would disable parallel query", which is
> not true after bb16aba5. Do we need to change that too?

Yeah, you're right.  That brings us to the attached.
From 19be76b8903ad6f179463bc058474338e9d53f8c Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Mon, 15 Mar 2021 17:08:25 +1300
Subject: [PATCH v2] Parallel query regression tests don't need SERIALIZABLE
 workaround.

SERIALIZABLE no longer inhibits parallelism, so the comment and
workaround in various regression tests are obsolete since commit
bb16aba5 (release 12).

Also fix a typo.

Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGJUaHeK%3DHLATxF1JOKDjKJVrBKA-zmbPAebOM0Se2FQRg%40mail.gmail.com
---
 src/test/regress/expected/aggregates.out      | 4 ++--
 src/test/regress/expected/explain.out         | 4 +---
 src/test/regress/expected/insert_parallel.out | 4 +---
 src/test/regress/expected/select_parallel.out | 4 +---
 src/test/regress/expected/write_parallel.out  | 6 ++----
 src/test/regress/sql/aggregates.sql           | 4 ++--
 src/test/regress/sql/explain.sql              | 4 +---
 src/test/regress/sql/insert_parallel.sql      | 4 +---
 src/test/regress/sql/select_parallel.sql      | 4 +---
 src/test/regress/sql/write_parallel.sql       | 6 ++----
 10 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 2c818d9253..1ae0e5d939 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2411,7 +2411,7 @@ ROLLBACK;
 -- Secondly test the case of a parallel aggregate combiner function
 -- returning NULL. For that use normal transition function, but a
 -- combiner function returning NULL.
-BEGIN ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
 CREATE FUNCTION balkifnull(int8, int8)
 RETURNS int8
 PARALLEL SAFE
@@ -2453,7 +2453,7 @@ SELECT balk(hundred) FROM tenk1;
 
 ROLLBACK;
 -- test coverage for aggregate combine/serial/deserial functions
-BEGIN ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
 SET parallel_setup_cost = 0;
 SET parallel_tuple_cost = 0;
 SET min_parallel_table_scan_size = 0;
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index dc7ab2ce8b..791eba8511 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -293,9 +293,7 @@ rollback;
 -- actually get (maybe none at all), we can't examine the "Workers" output
 -- in any detail.  We can check that it parses correctly as JSON, and then
 -- remove it from the displayed results.
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
 set parallel_tuple_cost=0;
diff --git a/src/test/regress/expected/insert_parallel.out b/src/test/regress/expected/insert_parallel.out
index d5fae79031..3599c21eba 100644
--- a/src/test/regress/expected/insert_parallel.out
+++ b/src/test/regress/expected/insert_parallel.out
@@ -52,9 +52,7 @@ insert into test_data select * from generate_series(1,10);
 --
 -- END: setup some tables and data needed by the tests.
 --
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
 set parallel_tuple_cost=0;
diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index 9b0c418db7..05ebcb284a 100644
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -3,9 +3,7 @@
 --
 create function sp_parallel_restricted(int) returns int as
   $$begin return $1; end$$ language plpgsql parallel restricted;
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
 set parallel_tuple_cost=0;
diff --git a/src/test/regress/expected/write_parallel.out b/src/test/regress/expected/write_parallel.out
index 0c4da2591a..77705f9a70 100644
--- a/src/test/regress/expected/write_parallel.out
+++ b/src/test/regress/expected/write_parallel.out
@@ -1,16 +1,14 @@
 --
 -- PARALLEL
 --
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
 set parallel_tuple_cost=0;
 set min_parallel_table_scan_size=0;
 set max_parallel_workers_per_gather=4;
 --
--- Test write operations that has an underlying query that is eligble
+-- Test write operations that has an underlying query that is eligible
 -- for parallel plans
 --
 explain (costs off) create table parallel_write as
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index f9579af19a..eb53668299 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -1002,7 +1002,7 @@ ROLLBACK;
 -- Secondly test the case of a parallel aggregate combiner function
 -- returning NULL. For that use normal transition function, but a
 -- combiner function returning NULL.
-BEGIN ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
 CREATE FUNCTION balkifnull(int8, int8)
 RETURNS int8
 PARALLEL SAFE
@@ -1035,7 +1035,7 @@ SELECT balk(hundred) FROM tenk1;
 ROLLBACK;
 
 -- test coverage for aggregate combine/serial/deserial functions
-BEGIN ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
 
 SET parallel_setup_cost = 0;
 SET parallel_tuple_cost = 0;
diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql
index c79116c927..f2eab030d6 100644
--- a/src/test/regress/sql/explain.sql
+++ b/src/test/regress/sql/explain.sql
@@ -83,9 +83,7 @@ rollback;
 -- in any detail.  We can check that it parses correctly as JSON, and then
 -- remove it from the displayed results.
 
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
 set parallel_tuple_cost=0;
diff --git a/src/test/regress/sql/insert_parallel.sql b/src/test/regress/sql/insert_parallel.sql
index 70ad31a087..8eb4e9564e 100644
--- a/src/test/regress/sql/insert_parallel.sql
+++ b/src/test/regress/sql/insert_parallel.sql
@@ -66,9 +66,7 @@ insert into test_data select * from generate_series(1,10);
 -- END: setup some tables and data needed by the tests.
 --
 
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index 5a01a98b26..d31e290ec2 100644
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -5,9 +5,7 @@
 create function sp_parallel_restricted(int) returns int as
   $$begin return $1; end$$ language plpgsql parallel restricted;
 
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
diff --git a/src/test/regress/sql/write_parallel.sql b/src/test/regress/sql/write_parallel.sql
index 78b479cedf..a5d63112c9 100644
--- a/src/test/regress/sql/write_parallel.sql
+++ b/src/test/regress/sql/write_parallel.sql
@@ -2,9 +2,7 @@
 -- PARALLEL
 --
 
--- Serializable isolation would disable parallel query, so explicitly use an
--- arbitrary other level.
-begin isolation level repeatable read;
+begin;
 
 -- encourage use of parallel plans
 set parallel_setup_cost=0;
@@ -13,7 +11,7 @@ set min_parallel_table_scan_size=0;
 set max_parallel_workers_per_gather=4;
 
 --
--- Test write operations that has an underlying query that is eligble
+-- Test write operations that has an underlying query that is eligible
 -- for parallel plans
 --
 explain (costs off) create table parallel_write as
-- 
2.30.1

Reply via email to