Standardize type of variable when extending Buffers
Hi,
This has already been discussed in [1]/messages/by-id/CAEudQAr_oWHpZk4uumZijYS362gp4KHAah-yUe08CQY4a4SsOQ@mail.gmail.com.
But I thought it best to start a new thread.
The commit 31966b1
<https://github.com/postgres/postgres/commit/31966b151e6ab7a6284deab6e8fe5faddaf2ae4c>
introduced the infrastructure to extend
buffers.
But the patch mixed types with int and uint32.
The correct type of the variable counter is uint32.
Fix by standardizing the int type to uint32.
patch attached.
best regards,
Ranier Vilela
[1]: /messages/by-id/CAEudQAr_oWHpZk4uumZijYS362gp4KHAah-yUe08CQY4a4SsOQ@mail.gmail.com
/messages/by-id/CAEudQAr_oWHpZk4uumZijYS362gp4KHAah-yUe08CQY4a4SsOQ@mail.gmail.com
Attachments:
0001-Standardize-type-of-extend_by-counter.patchapplication/octet-stream; name=0001-Standardize-type-of-extend_by-counter.patchDownload
From ee38e2523a50f63e5183f0d262d9777d5f5efd24 Mon Sep 17 00:00:00 2001
From: Ranier Vilela <ranier.vf@gmail.com>
Date: Fri, 7 Jul 2023 10:04:20 -0300
Subject: [PATCH] Standardize type of extend_by counter. The counter of
extend_by loop is mixed by int and uint32. Fix by standardizing from int to
uint32. To match limits of extend_by variable (uint32).
---
src/backend/storage/buffer/bufmgr.c | 6 +++---
src/backend/storage/buffer/localbuf.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index a7e3b9bb1d..526ec0d265 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -955,7 +955,7 @@ ExtendBufferedRelTo(ExtendBufferedWhat eb,
current_size = first_block + extended_by;
Assert(num_pages != 0 || current_size >= extend_to);
- for (int i = 0; i < extended_by; i++)
+ for (uint32 i = 0; i < extended_by; i++)
{
if (first_block + i != extend_to - 1)
ReleaseBuffer(buffers[i]);
@@ -1938,7 +1938,7 @@ ExtendBufferedRelShared(ExtendBufferedWhat eb,
* This needs to happen before we extend the relation, because as soon as
* we do, other backends can start to read in those pages.
*/
- for (int i = 0; i < extend_by; i++)
+ for (uint32 i = 0; i < extend_by; i++)
{
Buffer victim_buf = buffers[i];
BufferDesc *victim_buf_hdr = GetBufferDescriptor(victim_buf - 1);
@@ -2070,7 +2070,7 @@ ExtendBufferedRelShared(ExtendBufferedWhat eb,
io_start, extend_by);
/* Set BM_VALID, terminate IO, and wake up any waiters */
- for (int i = 0; i < extend_by; i++)
+ for (uint32 i = 0; i < extend_by; i++)
{
Buffer buf = buffers[i];
BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index f684862d98..a56e5ceb96 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -360,7 +360,7 @@ ExtendBufferedRelLocal(ExtendBufferedWhat eb,
relpath(eb.smgr->smgr_rlocator, fork),
MaxBlockNumber)));
- for (int i = 0; i < extend_by; i++)
+ for (uint32 i = 0; i < extend_by; i++)
{
int victim_buf_id;
BufferDesc *victim_buf_hdr;
@@ -416,7 +416,7 @@ ExtendBufferedRelLocal(ExtendBufferedWhat eb,
pgstat_count_io_op_time(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EXTEND,
io_start, extend_by);
- for (int i = 0; i < extend_by; i++)
+ for (uint32 i = 0; i < extend_by; i++)
{
Buffer buf = buffers[i];
BufferDesc *buf_hdr;
--
2.32.0.windows.1
On Fri, Jul 7, 2023 at 6:12 AM Ranier Vilela <ranier.vf@gmail.com> wrote:
Hi,
This has already been discussed in [1].
But I thought it best to start a new thread.The commit 31966b1 introduced the infrastructure to extend
buffers.
But the patch mixed types with int and uint32.
The correct type of the variable counter is uint32.Fix by standardizing the int type to uint32.
patch attached.
LGTM.
+CC Kyotaro, as they were involved in the previous discussion.
[1] /messages/by-id/CAEudQAr_oWHpZk4uumZijYS362gp4KHAah-yUe08CQY4a4SsOQ@mail.gmail.com
Best regards,
Gurjeet
http://Gurje.et
At Fri, 7 Jul 2023 11:29:16 -0700, Gurjeet Singh <gurjeet@singh.im> wrote in
On Fri, Jul 7, 2023 at 6:12 AM Ranier Vilela <ranier.vf@gmail.com> wrote:
Hi,
This has already been discussed in [1].
But I thought it best to start a new thread.The commit 31966b1 introduced the infrastructure to extend
buffers.
But the patch mixed types with int and uint32.
The correct type of the variable counter is uint32.Fix by standardizing the int type to uint32.
patch attached.
LGTM.
LGTM, too.
I don't think it will actually come to play, since I believe we won't
be expanding a relation by 16TB all at once. Nevertheless, I believe
keeping things tidy is a good habit to stick to.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Em seg., 10 de jul. de 2023 às 03:27, Kyotaro Horiguchi <
horikyota.ntt@gmail.com> escreveu:
At Fri, 7 Jul 2023 11:29:16 -0700, Gurjeet Singh <gurjeet@singh.im> wrote
inOn Fri, Jul 7, 2023 at 6:12 AM Ranier Vilela <ranier.vf@gmail.com>
wrote:
Hi,
This has already been discussed in [1].
But I thought it best to start a new thread.The commit 31966b1 introduced the infrastructure to extend
buffers.
But the patch mixed types with int and uint32.
The correct type of the variable counter is uint32.Fix by standardizing the int type to uint32.
patch attached.
LGTM.
LGTM, too.
Thanks Gurjeet and Kyotaro, for taking a look.
I don't think it will actually come to play, since I believe we won't
be expanding a relation by 16TB all at once. Nevertheless, I believe
keeping things tidy is a good habit to stick to.
Yeah, mainly because of copy-and-paste.
Also, compiler has to promote int to uint32, anyway.
regards,
Ranier Vilela
On 10.07.23 13:08, Ranier Vilela wrote:
Em seg., 10 de jul. de 2023 às 03:27, Kyotaro Horiguchi
<horikyota.ntt@gmail.com <mailto:horikyota.ntt@gmail.com>> escreveu:At Fri, 7 Jul 2023 11:29:16 -0700, Gurjeet Singh <gurjeet@singh.im
<mailto:gurjeet@singh.im>> wrote inOn Fri, Jul 7, 2023 at 6:12 AM Ranier Vilela <ranier.vf@gmail.com
<mailto:ranier.vf@gmail.com>> wrote:
Hi,
This has already been discussed in [1].
But I thought it best to start a new thread.The commit 31966b1 introduced the infrastructure to extend
buffers.
But the patch mixed types with int and uint32.
The correct type of the variable counter is uint32.Fix by standardizing the int type to uint32.
patch attached.
LGTM.
LGTM, too.
Thanks Gurjeet and Kyotaro, for taking a look.
committed
Em ter., 19 de set. de 2023 às 05:07, Peter Eisentraut <peter@eisentraut.org>
escreveu:
On 10.07.23 13:08, Ranier Vilela wrote:
Em seg., 10 de jul. de 2023 às 03:27, Kyotaro Horiguchi
<horikyota.ntt@gmail.com <mailto:horikyota.ntt@gmail.com>> escreveu:At Fri, 7 Jul 2023 11:29:16 -0700, Gurjeet Singh <gurjeet@singh.im
<mailto:gurjeet@singh.im>> wrote inOn Fri, Jul 7, 2023 at 6:12 AM Ranier Vilela <ranier.vf@gmail.com
<mailto:ranier.vf@gmail.com>> wrote:
Hi,
This has already been discussed in [1].
But I thought it best to start a new thread.The commit 31966b1 introduced the infrastructure to extend
buffers.
But the patch mixed types with int and uint32.
The correct type of the variable counter is uint32.Fix by standardizing the int type to uint32.
patch attached.
LGTM.
LGTM, too.
Thanks Gurjeet and Kyotaro, for taking a look.
committed
Thank you Peter.
best regards,
Ranier Vilela