Apologies for the noise, I overlooked a compiler warning. fixed.
-greg > On Mar 25, 2025, at 7:47 AM, Burd, Greg <gregb...@amazon.com> wrote: > > 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+postg...@gmail.com> wrote: >> >> On Thu, 6 Mar 2025 at 13:40, Burd, Greg <gregb...@amazon.com> wrote: >> >> >>> >>> >>> >>>> On Mar 5, 2025, at 6:39 PM, Matthias van de Meent >>>> <boekewurm+postg...@gmail.com> wrote: >>>> >>>> On Wed, 5 Mar 2025 at 18:21, Burd, Greg <gregb...@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. > > > 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. > > > > <v14-0001-Expand-HOT-update-path-to-include-expression-and.patch>
v15-0001-Expand-HOT-update-path-to-include-expression-and.patch
Description: v15-0001-Expand-HOT-update-path-to-include-expression-and.patch
