Re: BRIN range operator class - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: BRIN range operator class
Date
Msg-id 20150506213445.GA2523@alvh.no-ip.org
Whole thread Raw
In response to Re: BRIN range operator class  (Emre Hasegeli <emre@hasegeli.com>)
Responses Re: BRIN range operator class  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Patch for bug #12845 (GB18030 encoding)
Next
From: Tom Lane
Date:
Subject: Re: BRIN range operator class