Hi,

After doing some test to cover the code path in the PATCH 0001.
I have some suggestions for the 0002 testcase.


(1)
+                       /* Check parallel-safety of any expressions in the 
partition key */
+                       if (get_partition_col_attnum(pkey, i) == 0)
+                       {
+                               Node       *check_expr = (Node *) 
lfirst(partexprs_item);
+
+                               if (max_parallel_hazard_walker(check_expr, 
context))
+                               {
+                                       table_close(rel, lockmode);
+                                       return true;
+                               }

The testcase seems does not cover the above code(test when the table have 
parallel unsafe expression in the partition key).

Personally, I use the following sql to cover this:
-----
create table partkey_unsafe_key_expr_t (a int4, b name) partition by range 
((fullname_parallel_unsafe('',a::varchar)));
explain (costs off) insert into partkey_unsafe_key_expr_t select unique1, 
stringu1 from tenk1;
-----


(2)
I noticed that most of testcase test both (parallel safe/unsafe/restricted).
But the index expression seems does not test the parallel restricted.
How about add a testcase like:
-----
create or replace function fullname_parallel_restricted(f text, l text) returns 
text as $$
    begin
        return f || l;
    end;
$$ language plpgsql immutable parallel restricted;

create table names4(index int, first_name text, last_name text);
create index names4_fullname_idx on names4 
(fullname_parallel_restricted(first_name, last_name));

--
-- Test INSERT with parallel-restricted index expression
-- (should create a parallel plan)
--
explain (costs off) insert into names4 select * from names;
-----

(3)
+               /* Recursively check each partition ... */
+               pdesc = RelationGetPartitionDesc(rel);
+               for (i = 0; i < pdesc->nparts; i++)
+               {
+                       if (rel_max_parallel_hazard_for_modify(pdesc->oids[i],
+                                                                               
                                command_type,
+                                                                               
                                context,
+                                                                               
                                AccessShareLock))
+                       {
+                               table_close(rel, lockmode);
+                               return true;
+                       }
+               }

It seems we do not have a testcase to test (some parallel unsafe expression 
or.. in partition)
Hoe about add one testcase to test parallel unsafe partition ?

Best regards,
houzj



Reply via email to