Re: [PATCH] Resolve Parallel Hash Join Performance Issue

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: "Deng, Gang" <gang(dot)deng(at)intel(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Resolve Parallel Hash Join Performance Issue
Date: 2020-01-09 14:43:04
Message-ID: 4482.1578580984@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> Right, I see. The funny thing is that the match bit is not even used
> in this query (it's used for right and full hash join, and those
> aren't supported for parallel joins yet). Hmm. So, instead of the
> test you proposed, an alternative would be to use if (!parallel).
> That's a value that will be constant-folded, so that there will be no
> branch in the generated code (see the pg_attribute_always_inline
> trick). If, in a future release, we need the match bit for parallel
> hash join because we add parallel right/full hash join support, we
> could do it the way you showed, but only if it's one of those join
> types, using another constant parameter.

Can we base the test off the match type today, and avoid leaving
something that will need to be fixed later?

I'm pretty sure that the existing coding is my fault, and that it's
like that because I reasoned that setting the bit was too cheap
to justify having a test-and-branch around it. Apparently that's
not true anymore in a parallel join, but I have to say that it's
unclear why. In any case, the reasoning probably still holds good
in non-parallel cases, so it'd be a shame to introduce a run-time
test if we can avoid it.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-01-09 14:44:29 Re: logical decoding : exceeded maxAllocatedDescs for .spill files
Previous Message Christoph Berg 2020-01-09 14:38:45 Re: pgsql: Allow 'sslkey' and 'sslcert' in postgres_fdw user mappings