From: | Zhihong Yu <zyu(at)yugabyte(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-04-02 19:09:12 |
Message-ID: | CALNJ-vTtb7crR2=kx_q55xUDwR6Bo+m-sL1TJX9RNNaF4e+ZBQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
For v6-0003-Parallel-Hash-Full-Right-Outer-Join.patch
+ * current_chunk_idx: index in current HashMemoryChunk
The above comment seems to be better fit
for ExecScanHashTableForUnmatched(), instead
of ExecParallelPrepHashTableForUnmatched.
I wonder where current_chunk_idx should belong (considering the above
comment and what the code does).
+ while (hashtable->current_chunk_idx <
hashtable->current_chunk->used)
...
+ next = hashtable->current_chunk->next.unshared;
+ hashtable->current_chunk = next;
+ hashtable->current_chunk_idx = 0;
Each time we advance to the next chunk, current_chunk_idx is reset. It
seems current_chunk_idx can be placed inside chunk.
Maybe the consideration is that, with the current formation we save space
by putting current_chunk_idx field at a higher level.
If that is the case, a comment should be added.
Cheers
On Fri, Mar 5, 2021 at 5:31 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> On Tue, Mar 2, 2021 at 11:27 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
> wrote:
> > On Fri, Feb 12, 2021 at 11:02 AM Melanie Plageman
> > <melanieplageman(at)gmail(dot)com> wrote:
> > > I just attached the diff.
> >
> > Squashed into one patch for the cfbot to chew on, with a few minor
> > adjustments to a few comments.
>
> I did some more minor tidying of comments and naming. It's been on my
> to-do-list to update some phase names after commit 3048898e, and while
> doing that I couldn't resist the opportunity to change DONE to FREE,
> which somehow hurts my brain less, and makes much more obvious sense
> after the bugfix in CF #3031 that splits DONE into two separate
> phases. It also pairs obviously with ALLOCATE. I include a copy of
> that bugix here too as 0001, because I'll likely commit that first, so
> I rebased the stack of patches that way. 0002 includes the renaming I
> propose (master only). Then 0003 is Melanie's patch, using the name
> SCAN for the new match bit scan phase. I've attached an updated
> version of my "phase diagram" finger painting, to show how it looks
> with these three patches. "scan*" is new.
>
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2021-04-02 19:44:58 | Making wait events a bit more efficient |
Previous Message | Jacob Champion | 2021-04-02 18:18:44 | Re: Proposal: Save user's original authenticated identity for logging |