Re: Expanding HOT updates for expression and partial indexes

Matthias,

Rebased patch attached.

Changes in v14:
* UpdateContext now the location I've stored estate, resultRelInfo, etc.
* Reuse the result from the predicate on the partial index.

-greg

> On Mar 7, 2025, at 5:47 PM, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> wrote:
>
> On Thu, 6 Mar 2025 at 13:40, Burd, Greg <gregburd(at)amazon(dot)com> wrote:
>
>>
>>
>>> On Mar 5, 2025, at 6:39 PM, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> wrote:
>>>
>>> On Wed, 5 Mar 2025 at 18:21, Burd, Greg <gregburd(at)amazon(dot)com> wrote:
>>>
>>>> * augments IndexInfo only when needed for testing expressions and only once
>>>
>>>
>>> ExecExpressionIndexesUpdated seems to always loop over all indexes,
>>> always calling AttributeIndexInfo which always updates the fields in
>>> the IndexInfo when the index has only !byval attributes (e.g. text,
>>> json, or other such varlena types). You say it happens only once, have
>>> I missed something?
>>
>>
>> There's a test that avoids doing it more than once, [...]
>
>
> Is this that one?
>
> + if (indexInfo->ii_IndexAttrByVal)
> + return indexInfo;
>
> I think that test doesn't work consistently: a bitmapset * is NULL
> when no bits are set; and for some indexes no attribute will be byval,
> thus failing this early-exit even after processing.
>
> Another small issue with this approach is that it always calls and
> tests in EEIU(), while it's quite likely we would do better if we
> pre-processed _all_ indexes at once, so that we can have a path that
> doesn't repeatedly get into EEIU only to exit immediately after. It'll
> probably be hot enough to not matter much, but it's still cycles spent
> on something that we can optimize for in code.

I've changed this a bit, now in ExecOpenIndices() when there are expressions or predicates I augment the IndexInfo with information necessary to perform the tests in EEIU(). I've debated adding another bool to ExecOpenIndices() to indicate that we're opening indexes for the purpose of an update to avoid building that information in cases where we are not. Similar to the bool `speculative` on ExecOpenIndices() today. Thoughts?

>> I might widen this patch a bit to include support for testing equality of index tuples using custom operators when they exist for the index. In the use case I'm solving for we use a custom operator for equality that is not the same as a memcmp(). Do you have thoughts on that?
>
>
> I don't think that's a very great idea. From a certain point of view,
> you can see HOT as "deduplicating multiple tuple versions behind a
> single TID". Btree doesn't support deduplication for types that can
> have more than one representation of the same value so that e.g.
> '0.0'::numeric and '0'::numeric are both displayed correctly, even
> when they compare as equal according to certain equality operators.

Interesting, good point. Seems like it would require a new index AM function:
bool indexed_tuple_would_change()

I'll drop this for now, it seems out of scope for this patch set.