Hi Steven thanks for the review!
> I have two comments: > 1. Does table_beginscan_parallel_tidrange() need an assert of relid, > like what table_beginscan_parallel() did? > Assert(RelationGetRelid(relation) == pscan->phs_relid); In the v6 rebased patch, the assert has become: Assert(RelFileLocatorEquals(relation->rd_locator, pscan->phs_locator)); rather than: Assert(RelationGetRelid(relation) == pscan->phs_relid); table_beginscan_parallel_tidrange() already has the proper assert line similar to what table_beginscan_parallel() has. > 2. The new field phs_numblock in ParallelBlockTableScanDescData > structure has almost the same name as another field phs_nblocks. Would > you consider changing it to another name, for example, > phs_maxnumblocktoscan? I actually had a similar thought too, phs_nblocks and phs_numblock are very similar but are quite different. But I still left the name as phs_numblock because I want to keep it consistent (kind of) with the 'numBlks' used in heap_set_tidrange() in heapam.c. The comments besides their declaration should be enough to describe their differences without causing confusion. Best regards Cary