From: | Antonin Houska <ah(at)cybertec(dot)at> |
---|---|
To: | David Steele <david(at)pgmasters(dot)net> |
Cc: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Suspicious call of initial_cost_hashjoin() |
Date: | 2018-03-02 08:06:57 |
Message-ID: | 15738.1519978017@localhost |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
David Steele <david(at)pgmasters(dot)net> wrote:
> On 12/22/17 6:13 AM, Thomas Munro wrote:
> > On Fri, Dec 22, 2017 at 10:45 PM, Antonin Houska <ah(at)cybertec(dot)at> wrote:
> >> try_partial_hashjoin_path() passes constant true to for the parallel_hash
> >> argument of initial_cost_hashjoin(). Shouldn't it instead pass the
> >> parallel_hash argument that it receives?
> >
> > Thanks. Yeah. When initial_cost_hashjoin() calls
> > get_parallel_divisor() on a non-partial inner path I think it would
> > return 1.0, so no damage was done there, but when
> > ExecChooseHashTableSize() receives try_combined_work_mem == true it
> > might underestimate the number of batches required for a partial hash
> > join without parallel hash, because it would incorrectly assume that a
> > single batch join could use the combined work_mem budget. This was
> > quite well hidden because ExecHashTableCreate() calls
> > ExecChooseHashTableSize() again (rather than reusing the results from
> > planning time), so the bad nbatch estimate doesn't show up anywhere.
> Does this look right to you?
Yes, this is what I meant. The patch applies cleanly and the code compiles
well.
> If so, can you sign up as a reviewer and mark it Ready for Committer?
Done.
Actually I think it'd be nice if the "parallel_hash" argument was mentioned in
the header comment of initial_cost_hashjoin() function, but not sure this is
worth returning the patch to the author.
--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2018-03-02 08:16:32 | Re: Changing the autovacuum launcher scheduling; oldest table first algorithm |
Previous Message | Andres Freund | 2018-03-02 08:06:20 | Re: 2018-03 Commitfest Summary (Andres #4) |