On Wed, 24 Aug 2022 at 22:47, David Rowley <[email protected]> wrote:
> 5. "Refactor" (fix the code to make it better)
> I have some ideas on how to fix the two #5s, so I'm going to go and do that
> now.
I've attached a patch which I think improves the code in
gistRelocateBuildBuffersOnSplit() so that there's no longer a shadowed
variable. I also benchmarked this method in a tight loop and can
measure no performance change from getting the loop index this way vs
the old way.
This only fixes one of the #5s I mentioned. I ended up scraping my
idea to fix the shadowed 'i' in get_qual_for_range() as it became too
complex. The idea was to use list_cell_number() to find out how far
we looped in the forboth() loop. It turned out that 'i' was used in
the subsequent loop in "j = i;". The fix just became too complex and I
didn't think it was worth the risk of breaking something just to get
rid of the showed 'i'.
David
diff --git a/src/backend/access/gist/gistbuildbuffers.c
b/src/backend/access/gist/gistbuildbuffers.c
index eabf746018..c6c7dfe4c2 100644
--- a/src/backend/access/gist/gistbuildbuffers.c
+++ b/src/backend/access/gist/gistbuildbuffers.c
@@ -543,8 +543,7 @@ gistRelocateBuildBuffersOnSplit(GISTBuildBuffers *gfbb,
GISTSTATE *giststate,
GISTNodeBuffer *nodeBuffer;
BlockNumber blocknum;
IndexTuple itup;
- int splitPagesCount = 0,
- i;
+ int splitPagesCount = 0;
GISTENTRY entry[INDEX_MAX_KEYS];
bool isnull[INDEX_MAX_KEYS];
GISTNodeBuffer oldBuf;
@@ -595,11 +594,11 @@ gistRelocateBuildBuffersOnSplit(GISTBuildBuffers *gfbb,
GISTSTATE *giststate,
* Fill relocation buffers information for node buffers of pages
produced
* by split.
*/
- i = 0;
foreach(lc, splitinfo)
{
GISTPageSplitInfo *si = (GISTPageSplitInfo *) lfirst(lc);
GISTNodeBuffer *newNodeBuffer;
+ int i = foreach_current_index(lc);
/* Decompress parent index tuple of node buffer page. */
gistDeCompressAtt(giststate, r,
@@ -618,8 +617,6 @@ gistRelocateBuildBuffersOnSplit(GISTBuildBuffers *gfbb,
GISTSTATE *giststate,
relocationBuffersInfos[i].nodeBuffer = newNodeBuffer;
relocationBuffersInfos[i].splitinfo = si;
-
- i++;
}
/*
