From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Erik Nordström <erik(at)timescale(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Changed behavior in rewriteheap |
Date: | 2024-11-22 14:53:00 |
Message-ID: | 5234480a-f831-4bba-bb92-b9d34452b96a@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 22/11/2024 15:56, Erik Nordström wrote:
> Yes, it solves the issue so it looks good.
>
> Just a minor nit: the code uses both blokno as local variable for
> pending_writes[i].blkno and directly accessing pending_writes[i].blkno.
> Maybe it is better to just use the local variable. For example, change
>
> ++ b/src/backend/storage/smgr/bulk_write.c
> @@ -304,7 +304,8 @@ smgr_bulk_flush(BulkWriteState *bulkstate)
> }
>
> smgrextend(bulkstate->smgr, bulkstate->forknum,
> blkno, page, true);
> - bulkstate->relsize = pending_writes[i].blkno + 1;
> + bulkstate->relsize++;
> + Assert(bulkstate->relsize == blkno + 1);
>
> Just a suggestion.
Made that change and committed to master and REL_17_STABLE. I didn't
bother with the assertion though. Also I removed the 'pages_written'
field, it was not used for anything anymore.
Thanks!
--
Heikki Linnakangas
Neon (https://neon.tech)
From | Date | Subject | |
---|---|---|---|
Next Message | Erik Nordström | 2024-11-22 15:02:13 | Re: Changed behavior in rewriteheap |
Previous Message | Tomas Vondra | 2024-11-22 14:20:00 | Re: PoC: prefetching data between executor nodes (e.g. nestloop + indexscan) |