Re: Q about InsertIndexResult

Hi!

Insert function of any type of index must returns palloced InsertIndexResult
(contains blkno and offset).
This result is returned by index_insert (backend/access/indexam/indexam.c).
This function is called in 3 places:
./access/heap/tuptoaster.c
./catalog/indexing.c
./executor/execUtils.c

But in all this places returned value doesn't used, just pfree'ed (except
./access/heap/tuptoaster.c
where it's checked with NULL).

Q: why? why isn't it a bool value?
Is there some idea which I havn't see?

--
Teodor Sigaev
teodor(at)stack(dot)net

Teodor Sigaev <teodor(at)stack(dot)net> writes:
> [ why do we have InsertIndexResult and not just a bool? ]

Good question. Perhaps it was used once upon a time? That API has been
like that since Berkeley days.

I can't offhand see a good reason to return the index tuple's tid.
There isn't any legitimate reason for anything outside the index AM
to be doing anything directly with the index tuple.

I dunno if it's worth the trouble to change it just to save one palloc
per insert, though. If we ever decided that there was some other piece
of information that the index AM should return, we'd have to change
right back to returning a struct...

regards, tom lane

> I can't offhand see a good reason to return the index tuple's tid.
One reason why existing interface is not good:
Who say, that for one heap tuple should exists only one index tuple?
For example, Oleg and Vadim Mikheev had discussian pair years ago about
indexing arrays by B-tree: for each heap tuple stores one index tuple per
element of array.

> There isn't any legitimate reason for anything outside the index AM
> to be doing anything directly with the index tuple.
> I dunno if it's worth the trouble to change it just to save one palloc
> per insert, though. If we ever decided that there was some other piece
> of information that the index AM should return, we'd have to change
> right back to returning a struct...
Agreed.

--
Teodor Sigaev
teodor(at)stack(dot)net

Teodor Sigaev <teodor(at)stack(dot)net> writes:
> One reason why existing interface is not good:
> Who say, that for one heap tuple should exists only one index tuple?

Good point. Is that reason enough to change it? Not sure. The index
AM doesn't really have to bother with telling the truth in the
InsertIndexResult it returns ...

regards, tom lane