Re: Partition-wise join for join between (declaratively) partitioned tables

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Partition-wise join for join between (declaratively) partitioned tables
Date: 2017-09-20 09:43:40
Message-ID: CAFjFpRcQV74TMERjbikBrY7TLVb9LSLXRusn9MJtfiwPbw3S6w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 20, 2017 at 9:44 AM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>
> The main areas of uncovered lines are: code in
> get_wholerow_ref_from_convert_row_type() and code that calls it, and
> per node type cases in reparameterize_path_by_child(). It seems like
> the former could use a test case, and I wonder if there is some way we
> could write "flat-copy and then apply recursively to all subpaths"
> code like this without having to handle these cases explicitly. There
> are a couple of other tiny return cases other than just sanity check
> errors which it might be interesting to hit too.

Under the debugger I checked that the test in partition_join.sql
-- left outer join, with whole-row reference
EXPLAIN (COSTS OFF)
SELECT t1, t2 FROM prt1 t1 LEFT JOIN prt2 t2 ON t1.a = t2.b WHERE t1.b
= 0 ORDER BY t1.a, t2.b;
SELECT t1, t2 FROM prt1 t1 LEFT JOIN prt2 t2 ON t1.a = t2.b WHERE t1.b
= 0 ORDER BY t1.a, t2.b;
covers get_wholerow_ref_from_convert_row_type(). But it doesn't cover
a couple of lines in the case of nested ConvertRowTypeExpr in that
function. We can add/modify a testcase in multi-level partitioned
table section to cover that.

reparameterize_path_by_child() coverage is hard. It would require that
many different kinds of paths survive in lower joins in the join tree.
It's hard to come up with queries that would do that with limited
amount of data and a reasonable number of tests. Me and Thomas
discussed about his suggestion about "flat-copy and then apply
recursively to all subpaths" which he sees as a path tree mutator. It
won't improve the test coverage. Like expression_tree_mutator() path
mutation is not that widely used phenomenon, so we do not yet know
what should be the characteristics of a path mutator could be. In case
we see more of path mutation code in future, it's an idea worth
considering.

>
> 2. What queries in the 0008 patch are hitting lines that 0007 doesn't hit?
>
> I thought about how to answer questions like this and came up with a
> shell script that (1) makes computers run really hot for quite a long
> time and (2) tells you which blocks of SQL hit which lines of C.
> Please find attached the shell script and its output. The .sql files
> have been annotated with "block" numbers (blocks being chunks of SQL
> stuff separated by blank lines), and the C files annotated with
> references to those block numbers where A<n> = block <n>
> partition_join.sql and B<n> = block <n> in partition_join_extras.sql.
>
> Then to find lines that B queries hit but A queries don't and know
> which particular queries hit them, you might use something like:
>
> grep -v 'SQL blocks: .*A[0-9]' < joinpath.c.aggregated_coverage | \
> grep 'SQL blocks: .*B[0-9]'
>

Thanks for this. It generates a lot of output (970 lines over all the
coverage files). It will take some time for getting anything
meaningful out of this. May be there's some faster way by looking at
the lines that are covered by B but not A. BTW, I checked those lines
to see if there could be any bug there. But I don't see what could go
wrong with those lines.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2017-09-20 09:50:46 Re: src/test/subscription/t/002_types.pl hanging on particular environment
Previous Message Andrey Borodin 2017-09-20 09:43:18 Re: Allow GiST opcalsses without compress\decompres functions