MIME-Version: 1.0
References: <78574B24-BE0A-42C5-8075-3FA9FA63B8FC@amazon.com>
 <CAEze2WjjOg+gE1VUZ2Omd-26MniaY6-UJghqzLZMHpVkDEUy8w@mail.gmail.com>
 <DBA06D41-32C8-45D5-A3FA-15A412692351@amazon.com>
 <CAEze2WgyAUtW64eP8uxZmHUy-RfBtuiKaUybvi=Jc3bwgg3g4Q@mail.gmail.com>
 <EA519A89-282D-4535-88C6-C79588FF1DA8@amazon.com>
 <CAEze2WjCJokQbBOQqK0qJFw0H=cu27XB=hrgQOhy9TQ=41RPmg@mail.gmail.com>
 <238EEE41-B206-4590-8C20-DA52C25A2291@amazon.com>
 <D50C3248-FF76-4E6C-B891-B6E147F56A44@amazon.com>
 <51C77060-059F-4BB3-8EAF-83F08656F6D2@amazon.com>
 <CAEze2WjJqjyJk-7XBtiYBovJMg5tLZkS5MGii56oCOa1Z+MKWQ@mail.gmail.com>
 <B271EE64-84D8-42C2-AACE-441C22CB3587@amazon.com>
In-Reply-To: <B271EE64-84D8-42C2-AACE-441C22CB3587@amazon.com>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Fri, 7 Mar 2025 23:47:45 +0100
Message-ID: 
 <CAEze2WgasQk4Nwod=YyqdmGT=zYWGf8=Mne7EN3e7ygmnj9oaA@mail.gmail.com>
Subject: Re: Expanding HOT updates for expression and partial indexes
To: "Burd, Greg" <gregburd@amazon.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAEze2WgasQk4Nwod%3DYyqdmGT%3DzYWGf8%3DMne7EN3e7ygmnj9oaA%40mail.gmail.com>
Precedence: bulk

On Thu, 6 Mar 2025 at 13:40, Burd, Greg <gregburd@amazon.com> wrote:
>
> > On Mar 5, 2025, at 6:39=E2=80=AFPM, Matthias van de Meent <boekewurm+po=
stgres@gmail.com> wrote:
> >
> > On Wed, 5 Mar 2025 at 18:21, Burd, Greg <gregburd@amazon.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.

> >> * retains existing summarized index HOT update logic
> >
> > Great, thanks!
> >
> > Kind regards,
> >
> > Matthias van de Meent
> > Neon (https://neon.tech)
>
> 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.

So, I don't think that's worth investing time into right now. Maybe in
the future if there are new discoveries about what we can and cannot
deduplicate, but I don't think it should be part of an MVP or 1.0.


Kind regards,

Matthias van de Meent