From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
---|---|
To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Tomas Vondra <tv(at)fuzzy(dot)cz> |
Subject: | Re: Can rs_cindex be < 0 for bitmap heap scans? |
Date: | 2024-10-24 13:40:54 |
Message-ID: | CAAKRu_Z0VFTXjJ88=7=ex=dnC1MmjorRjmSysjdKdRk8OGAZJg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the reply, Dilip!
On Thu, Oct 24, 2024 at 12:50 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Thu, Oct 24, 2024 at 3:45 AM Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote:
>>
>> HeapScanDescData->rs_cindex (the current index into the array of
>> visible tuples stored in the heap scan descriptor) is used for
>> multiple scan types, but for bitmap heap scans, AFAICT, it should
>> never be < 0.
>
> You are right it can never be < 0 in this case at least.
Cool, thanks for confirming. I will commit this change then.
> In fact you don't need to explicitly set it to 0 in initscan[1], because before calling heapam_scan_bitmap_next_tuple() we must call heapam_scan_bitmap_next_block() and this function is initializing this to 0 (hscan->rs_cindex = 0;). Anyway no object even if you prefer to initialize in initscan(), just the point is that we are already doing it for each block before fetching tuples from the block.
I am inclined to still initialize it to 0 in initscan(). I was
refactoring the bitmap heap scan code to use the read stream API and
after moving some things around, this field was no longer getting
initialized in heapam_scan_bitmap_next_block(). While that may not be
a good reason to change it in this patch, most of the other fields in
the heap scan descriptor (like rs_cbuf and rs_cblock) are
re-initialized in initscan(), so it seems okay to do that there even
though it isn't strictly necessary on master.
- Melanie
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2024-10-24 13:52:43 | Re: Add support to TLS 1.3 cipher suites and curves lists |
Previous Message | hugo | 2024-10-24 13:07:36 | Useless field ispartitioned in CreateStmtContext |