From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Melanie Plageman <melanieplageman(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-25 04:41:00 |
Message-ID: | CAFiTN-vgMWeqPeXZuH9bM9JwWGLFGgTUiJt2k1rrOUTQmzQB=A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Oct 24, 2024 at 7:11 PM Melanie Plageman <melanieplageman(at)gmail(dot)com>
wrote:
> 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.
>
+1
>
> > 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.
>
make sense
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Andrei Lepikhov | 2024-10-25 04:51:36 | Re: cache lookup failed when \d t concurrent with DML change column data type |
Previous Message | Tatsuo Ishii | 2024-10-25 04:04:53 | Re: Row pattern recognition |