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.mu...@gmail.com> wrote: > On Tue, Mar 2, 2021 at 11:27 PM Thomas Munro <thomas.mu...@gmail.com> > wrote: > > On Fri, Feb 12, 2021 at 11:02 AM Melanie Plageman > > <melanieplage...@gmail.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. >
