Re: Making all nbtree entries unique by having heap TIDs participate in comparisons

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Claudio Freire <klaussfreire(at)gmail(dot)com>, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>, "Andrey V(dot) Lepikhov" <a(dot)lepikhov(at)postgrespro(dot)ru>
Subject: Re: Making all nbtree entries unique by having heap TIDs participate in comparisons
Date: 2019-03-18 11:59:45
Message-ID: df72dc8c-8757-5f09-8607-54535865f83d@iki.fi
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 18/03/2019 02:59, Peter Geoghegan wrote:
> On Sat, Mar 16, 2019 at 1:05 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>> Actually, how about "rootsearch", or "rootdescend"? You're supposed to
>>> hyphenate "re-find", and so it doesn't really work as a function
>>> argument name.
>>
>> Works for me.
>
> Attached is v18 of patch series, which calls the new verification
> option "rootdescend" verification.

Thanks!

I'm getting a regression failure from the 'create_table' test with this:

> --- /home/heikki/git-sandbox/postgresql/src/test/regress/expected/create_table.out 2019-03-11 14:41:41.382759197 +0200
> +++ /home/heikki/git-sandbox/postgresql/src/test/regress/results/create_table.out 2019-03-18 13:49:49.480249055 +0200
> @@ -413,18 +413,17 @@
> c text,
> d text
> ) PARTITION BY RANGE (a oid_ops, plusone(b), c collate "default", d collate "C");
> +ERROR: function plusone(integer) does not exist
> +HINT: No function matches the given name and argument types. You might need to add explicit type casts.

Are you seeing that?

Looking at the patches 1 and 2 again:

I'm still not totally happy with the program flow and all the conditions
in _bt_findsplitloc(). I have a hard time following which codepaths are
taken when. I refactored that, so that there is a separate copy of the
loop for V3 and V4 indexes. So, when the code used to be something like
this:

_bt_findsplitloc(...)
{
...

/* move right, if needed */
for(;;)
{
/*
* various conditions for when to stop. Different conditions
* apply depending on whether it's a V3 or V4 index.
*/
}

...
}

it is now:

_bt_findsplitloc(...)
{
...

if (heapkeyspace)
{
/*
* If 'checkingunique', move right to the correct page.
*/
for (;;)
{
...
}
}
else
{
/*
* Move right, until we find a page with enough space or "get
* tired"
*/
for (;;)
{
...
}
}

...
}

I think this makes the logic easier to understand. Although there is
some commonality, the conditions for when to move right on a V3 vs V4
index are quite different, so it seems better to handle them separately.
There is some code duplication, but it's not too bad. I moved the common
code to step to the next page to a new helper function, _bt_stepright(),
which actually seems like a good idea in any case.

See attached patches with those changes, plus some minor comment
kibitzing. It's still failing the 'create_table' regression test, though.

- Heikki

PS. The commit message of the first patch needs updating, now that
BTInsertState is different from BTScanInsert.

Attachment Content-Type Size
v18-heikki-0001-Refactor-nbtree-insertion-scankeys.patch text/x-patch 64.5 KB
v18-heikki-0002-Make-heap-TID-a-tiebreaker-nbtree-index-column.patch text/x-patch 157.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2019-03-18 12:33:12 Re: Add exclusive backup deprecation notes to documentation
Previous Message Rafia Sabih 2019-03-18 11:42:23 Re: [HACKERS] CLUSTER command progress monitor