[PATCH] Use binaryheap_* macro where appropriate
Started by Aleksander Alekseev7 months ago3 messages
Hi,
I noticed several places where we access `struct binaryheap` directly
instead of using binaryheap_size() and binaryheap_empty() which
doesn't seem right. Here is the patch.
--
Best regards,
Aleksander Alekseev
Attachments:
v1-0001-Use-binaryheap_-macro-where-appropriate.patchapplication/octet-stream; name=v1-0001-Use-binaryheap_-macro-where-appropriate.patchDownload
From e732a1b6e6c1ba58cdb0ce5f78edc610c54725d0 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Thu, 26 Jun 2025 11:41:26 +0300
Subject: [PATCH v1] Use binaryheap_* macro where appropriate
---
src/backend/postmaster/pgarch.c | 12 ++++++------
src/backend/replication/logical/reorderbuffer.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 7e622ae4bd2..78e39e5f866 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -718,15 +718,15 @@ pgarch_readyXlog(char *xlog)
/*
* Store the file in our max-heap if it has a high enough priority.
*/
- if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
+ if (binaryheap_size(arch_files->arch_heap) < NUM_FILES_PER_DIRECTORY_SCAN)
{
/* If the heap isn't full yet, quickly add it. */
- arch_file = arch_files->arch_filenames[arch_files->arch_heap->bh_size];
+ arch_file = arch_files->arch_filenames[binaryheap_size(arch_files->arch_heap)];
strcpy(arch_file, basename);
binaryheap_add_unordered(arch_files->arch_heap, CStringGetDatum(arch_file));
/* If we just filled the heap, make it a valid one. */
- if (arch_files->arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN)
+ if (binaryheap_size(arch_files->arch_heap) == NUM_FILES_PER_DIRECTORY_SCAN)
binaryheap_build(arch_files->arch_heap);
}
else if (ready_file_comparator(binaryheap_first(arch_files->arch_heap),
@@ -744,21 +744,21 @@ pgarch_readyXlog(char *xlog)
FreeDir(rldir);
/* If no files were found, simply return. */
- if (arch_files->arch_heap->bh_size == 0)
+ if (binaryheap_empty(arch_files->arch_heap))
return false;
/*
* If we didn't fill the heap, we didn't make it a valid one. Do that
* now.
*/
- if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
+ if (binaryheap_size(arch_files->arch_heap) < NUM_FILES_PER_DIRECTORY_SCAN)
binaryheap_build(arch_files->arch_heap);
/*
* Fill arch_files array with the files to archive in ascending order of
* priority.
*/
- arch_files->arch_files_size = arch_files->arch_heap->bh_size;
+ arch_files->arch_files_size = binaryheap_size(arch_files->arch_heap);
for (int i = 0; i < arch_files->arch_files_size; i++)
arch_files->arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index c4299c76fb1..7b4e8629553 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1415,7 +1415,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
int32 off;
/* nothing there anymore */
- if (state->heap->bh_size == 0)
+ if (binaryheap_empty(state->heap))
return NULL;
off = DatumGetInt32(binaryheap_first(state->heap));
--
2.49.0
Re: [PATCH] Use binaryheap_* macro where appropriate
On Thu, Jun 26, 2025 at 01:18:10PM +0300, Aleksander Alekseev wrote:
I noticed several places where we access `struct binaryheap` directly
instead of using binaryheap_size() and binaryheap_empty() which
doesn't seem right. Here is the patch.
Seems reasonable. I'll plan on committing this once v19 is open for
business.
--
nathan