From: | Cary Huang <cary(dot)huang(at)highgo(dot)ca> |
---|---|
To: | "Steven Niu" <niushiji(at)gmail(dot)com> |
Cc: | "pgsql-hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Support tid range scan in parallel? |
Date: | 2025-06-27 22:07:53 |
Message-ID: | 197b36ecfd5.c65457ca723543.3160355008280014188@highgo.ca |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Nathan Bossart | 2025-06-27 22:32:29 | Re: pg_get_multixact_members not documented |
Previous Message | Peter Geoghegan | 2025-06-27 21:35:50 | Re: Making Row Comparison NULL row member handling more robust during skip scans |