On 2 Jun 2026, at 00:12, Álvaro Herrera <[email protected]> wrote:
How about something like this? It makes your test case throw an error
instead of failing the assertion, which I suppose is an improvement.
The patch is a bit noisy because I moved more code than the minimum
necessary; but the gist of it is that we allocate RepackDecodingState in
repack_startup(), then have repack_setup_logical_decoding() fill in a
magic number, which we later check in repack_begin_txn(). This is a bit
wasteful, because we have to do that check once for each and every
transaction; however I see no other callback that would let us do this
kind of check after the slot is created but before we start to consume
from it.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are." -- Charles J. Sykes' advice to teenagers
<0001-Have-RepackDecodingState-carry-a-magic-number.patch>
Yes, I agree that returning an error to the user makes sense.
But does the error message need to be that detailed? Perhaps something like
"ERROR: wrong magic number in "pgrepack" decoder plugin"
would be sufficient.
Nevertheless, I tested the patch and can confirm that there are no assertion failures anymore.
I also ran it under ASAN and did not observe any issues.
Would it make sense to add a test for this case from the bug report?