Re: rewrite HeapSatisfiesHOTAndKey

On Thu, Dec 29, 2016 at 4:50 AM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Pursuant to my comments at
> https://www.postgresql.org/message-id/20161223192245.hx4rbrxbrwtgwj6i@alvherre.pgsql
> and because of a stupid bug I found in my indirect indexes patch, I
> rewrote (read: removed) HeapSatisfiesHOTAndKey. The replacement
> function HeapDetermineModifiedColumns returns a bitmapset with a bit set
> for each modified column, for those columns that are listed as
> "interesting" -- currently that set is the ID columns, the "key"
> columns, and the indexed columns. The new code is much simpler, at the
> expense of a few bytes of additional memory used during heap_update().
>
> All tests pass.
>
> Both WARM and indirect indexes should be able to use this infrastructure
> in a way that better serves them than the current HeapSatisfiesHOTAndKey
> (both patches modify that function in a rather ugly way). I intend to
> get this pushed shortly unless objections are raised.
>
> The new coding prevents stopping the check early as soon as the three
> result booleans could be determined.
>

I think there is some chance that such a change could induce
regression for the cases when there are many index columns or I think
even when index is on multiple columns (consider index is on first and
eight column in a ten column table). The reason for such a suspicion
is that heap_getattr() is not so cheap that doing it multiple times is
free. Do you think it is worth to do few tests before committing the
patch?

Noticed below comment in interesting-attrs-2.patch
+ * are considered the "key" of rows in the table, and columns that are
+ * part of indirect indexes.

Is it right to mention about indirect indexes in above comment
considering indirect indexes are still not part of core code?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com