public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mihail Nikalayeu <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Antonin Houska <[email protected]>
Cc: Srinath Reddy Sadipiralla <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Robert Treat <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Mon, 6 Apr 2026 12:14:57 +0200
Message-ID: <CADzfLwUB=iafkv2dR-9oFgjAANRZE2icUJQyt-zLpcs_T_ug8w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
Hello!
On Sun, Apr 5, 2026 at 10:41 PM Alvaro Herrera <[email protected]> wrote:
> So here's a v55 version of the base REPACK patches that I'm feeling
> comfortable calling very close to committable.
Some comments for v55.
repack.c:2725
if (!VARATT_IS_EXTERNAL(varlen))
continue;
I think it should be VARATT_IS_EXTERNAL_INDIRECT - the same as in pgrepack:244.
Also, after
natt_ext--;
I think it worth to add
Assert(natt_ext >= 0);
or Assert(natt_ext == 0); but in another place.
Or exit early with
for (int i = 0; i < desc->natts && natt_ext > 0; i++)
----------------------------
repack.c:2587
table_tuple_insert(rel, slot, GetCurrentCommandId(true),
HEAP_INSERT_NO_LOGICAL, NULL);
More idiomatic to use TABLE_INSERT_NO_LOGICAL instead.
----------------------------
repack.c:2696
ExecForceStoreHeapTuple(tup, slot, false);
AFAIU there is a memory leak here. Memory allocated above (for tuple)
is not freed in any way, because shouldFree == false.
Also, ExecClearTuple (tts_virtual_clear for virtual tuples) requires
TTS_SHOULDFREE to be set to free anything.
-------------------------
grab ShareUpdateExclusiveLock (jsut like VACUUM
typo in "just"
--------------------------
"If the identity index is not set due to replica identity being, PK"
Missing "FULL" after "being"?
-------------------------
Commit message:
"intial copy" -> "initial copy"
"backed performing REPACK" -> "backend performing REPACK"
Best regards,
Mikhail.
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <CADzfLwUB=iafkv2dR-9oFgjAANRZE2icUJQyt-zLpcs_T_ug8w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox