> On 8 Oct 2024, at 17:05, Tomas Vondra <tomas@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
befastest 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
choosingLSN 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.
