Hi!
Nice feature!
> On 8 Jun 2025, at 17:39, Arseniy Mukhin <arseniy.mukhin.dev@gmail.com> wrote:
>
> <v2-0001-brin-refactoring.patch>
+#define BRIN_MAX_PAGES_PER_RANGE 131072
I'd personally prefer some words on where does this limit come from. I'm not a BRIN pro, just curious.
Or, at least, maybe we can use a form 128 * 1024, if it's just a sane limit.
> On 8 Jun 2025, at 17:39, Arseniy Mukhin <arseniy.mukhin.dev@gmail.com> wrote:
> <v2-0002-amcheck-brin-support.patch>
A bit more detailed commit message would be very useful.
The test coverage is very decent. The number of possible corruptions in tests is impressive. I don't really have an
experiencewith BRIN to say how different they are, but I want to ask if you are sure that these types of corruption
willbe portable across big-endian machines and such stuff.
Copyright year in all new files should be 2025.
Some documentation about brin_index_check() would be handy, especially about its parameters. Perhaps, somewhere near
gin_index_check()in amcheck.sgml.
brin_check_ereport() reports ERRCODE_DATA_CORRUPTED. We should distinguish between ERRCODE_INDEX_CORRUPTED which is a
stormbringerand ERRCODE_DATA_CORRUPTED which is an actual disaster.
If it's not very difficult - it would be great to use read_stream infrastructure. See btvacuumscan() in nbtree.c
callingread_stream_begin_relation() for example. We cannot use it in logical scans in B-tree\GiST\GIN, but maybe BRIN
cantake some advantage of this new shiny technology.
+ state->revmapbuf = ReadBufferExtended(idxrel, MAIN_FORKNUM, state->revmapBlk, RBM_NORMAL,
+ state->checkstrategy);
+ LockBuffer(state->revmapbuf, BUFFER_LOCK_SHARE);
// usage of state->revmapbuf
+ LockBuffer(state->revmapbuf, BUFFER_LOCK_UNLOCK);
// more usage of state->revmapbuf
+ ReleaseBuffer(state->revmapbuf);
I hope you know what you are doing. BRIN concurrency is not known to me at all.
That's all for first pass through patches. Thanks for working on it!
Best regards, Andrey Borodin.
