Re: [PATCH] Don't block HOT update by BRIN index

Hi,

I've polished the patch a bit, with the goal to get it committed. I've
added the new amhotblocking flag to indexam.sgml (and I'm wondering if
there's another place in docs for more details).

But then I realized there's an issue in handling the predicate. Consider
this example:

drop table if exists t;
create table t (a int, b int);
insert into t values (1, 100);
create index on t using brin (b) where a = 2;

update t set a = 2;

explain analyze select * from t where a = 2 and b = 100;
set enable_seqscan = off;
explain analyze select * from t where a = 2 and b = 100;

With the previous version of the patch, the explains are this:

QUERY PLAN
----------------------------------------------------------------------
Seq Scan on t (cost=0.00..1.01 rows=1 width=8)
(actual time=0.006..0.007 rows=1 loops=1)
Filter: ((a = 2) AND (b = 100))
Planning Time: 0.040 ms
Execution Time: 0.018 ms
(4 rows)

QUERY PLAN
----------------------------------------------------------------------
Bitmap Heap Scan on t (cost=12.03..16.05 rows=1 width=8)
(actual time=0.007..0.009 rows=0 loops=1)
Recheck Cond: ((b = 100) AND (a = 2))
-> Bitmap Index Scan on t_b_idx (cost=0.00..12.03 rows=1 width=0)
(actual time=0.006..0.006 rows=0 loops=1)
Index Cond: (b = 100)
Planning Time: 0.041 ms
Execution Time: 0.026 ms
(6 rows)

Notice that the second plan (using the brin index) produces 0 rows,
which is obviously wrong. Clearly, the index was not updated.

I think this is caused by simple thinko in RelationGetIndexAttrBitmap,
which did this:

/* Collect all attributes in the index predicate, too */
if (indexDesc->rd_indam->amhotblocking)
pull_varattnos(indexPredicate, 1, &hotblockingattrs);

I think this is wrong - we should not ignore the predicate based on
amhotblocking, because then we'll fail to notice an update making the
tuple match the index predicate (as in the example).

The way I understand heap_update() it does not try to determine if the
update makes the tuple indexable, it just disables HOT when it might
happen. The attached patch just calls pull_varattnos every time.

I wonder if we might be a bit smarter about the predicates vs. HOT, and
disable HOT only when the tuple becomes indexable after the update.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company