> On 8 Oct 2024, at 17:05, Tomas Vondra <to...@vondra.me> wrote:
>
> Here's an updated patch adding the queryid.
I've took another round of looking through the patch.
Everything I see seems correct to me. It just occurred to me that we will have:
buffered build, parallel build, sorting build. All 3 aiming to speed things up.
I really hope that we will find a way to parallel sorting build, because it
will be fastest for sure.
Currently we have some instances of such code...or similar... or related code.
if (is_build)
{
if (is_parallel)
recptr = GetFakeLSNForUnloggedRel();
else
recptr = GistBuildLSN;
}
else
{
if (RelationNeedsWAL(rel))
{
recptr = actuallyWriteWAL();
}
else
recptr = gistGetFakeLSN(rel);
}
// Use recptr
In previous review I was proponent of not adding arguments to gistGetFakeLSN().
But now I see that now logic of choosing LSN source is sprawling across various
places...
Now I do not have a strong point of view on this. Do you think if something
like following would be clearer?
if (!is_build && RelationNeedsWAL(rel))
{
recptr = actuallyWriteWAL();
}
else
recptr = gistGetFakeLSN(rel, is_build, is_parallel);
Just as an idea.
I'm mostly looking on GiST-specific parts of the patch, while things around
entering parallel mode seems much more complicated. But as far as I can see,
large portions of this code are taken from B-tree\BRIN.
Best regards, Andrey Borodin.
