Re: Parallel Full Hash Join

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Parallel Full Hash Join
Date: 2021-02-11 22:02:18
Message-ID: 20210211220218.GA384835@goldwasser
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 29, 2020 at 03:28:12PM +1300, Thomas Munro wrote:
> I had some feedback I meant to
> post in November but didn't get around to:
>
> * PHJ_BATCH_PROBING -- all probe
> - * PHJ_BATCH_DONE -- end
> +
> + * PHJ_BATCH_DONE -- queries not requiring inner fill done
> + * PHJ_BATCH_FILL_INNER_DONE -- inner fill completed, all queries done
>
> Would it be better/tidier to keep _DONE as the final phase? That is,
> to switch around these two final phases. Or does that make it too
> hard to coordinate the detach-and-cleanup logic?

I updated this to use your suggestion. My rationale for having
PHJ_BATCH_DONE and then PHJ_BATCH_FILL_INNER_DONE was that, for a worker
attaching to the batch for the first time, it might be confusing that it
is in the PHJ_BATCH_FILL_INNER state (not the DONE state) and yet that
worker still just detaches and moves on. It didn't seem intuitive.
Anyway, I think that is all sort of confusing and unnecessary. I changed
it to PHJ_BATCH_FILLING_INNER -- then when a worker who hasn't ever been
attached to this batch before attaches, it will be in the
PHJ_BATCH_FILLING_INNER phase, which it cannot help with and it will
detach and move on.

>
> +/*
> + * ExecPrepHashTableForUnmatched
> + * set up for a series of ExecScanHashTableForUnmatched calls
> + * return true if this worker is elected to do the
> unmatched inner scan
> + */
> +bool
> +ExecParallelPrepHashTableForUnmatched(HashJoinState *hjstate)
>
> Comment name doesn't match function name.

Updated -- and a few other comment updates too.

I just attached the diff.

Attachment Content-Type Size
v4-0002-Update-comments-and-phase-naming.patch text/x-diff 3.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2021-02-11 22:55:45 Re: pg_cryptohash_final possible out-of-bounds access (per Coverity)
Previous Message Ranier Vilela 2021-02-11 21:56:13 Re: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)