Re: BRIN range operator class

I again have to refuse the notion that removing the assert-only block
without any replacement is acceptable. I just spent a lot of time
tracking down what turned out to be a bug in your patch 07:

/* Adjust maximum, if B's max is greater than A's max */
- needsadj = FunctionCall2Coll(minmax_get_procinfo(bdesc, attno,
- PROCNUM_GREATER),
- colloid, col_b->bv_values[1], col_a->bv_values[1]);
+ frmg = minmax_get_strategy_procinfo(bdesc, attno, attr->atttypid,
+ BTGreaterStrategyNumber);
+ needsadj = FunctionCall2Coll(frmg, colloid, col_b->bv_values[0],
+ col_a->bv_values[0]);

Note the removed lines use array index 1, while the added lines use
array index 0. The only reason I noticed this is because I applied this
patch without the others and saw the assertion fire; how would I have
noticed the problem had I just removed it?

Let's think together and try to find a reasonable way to get the union
procedures tested regularly. It is pretty clear that having them run
only when the race condition occurs is not acceptable; bugs go
unnoticed.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services