Hi, I think I found the issue - it's kinda obvious, really. We need to consider the timezone, because the "time" parts alone may be sorted differently. The attached patch should fix this, and it also fixes a similar issue in the inet data type.
As for why the regression tests did not catch this, it's most likely because the data is likely generated in "nice" ordering, or something like that. I'll see if I can tweak the ordering to trigger these issues reliably, and I'll do a bit more randomized testing. There's also the question of rounding errors, which I think might cause random assert failures (but in practice it's harmless, in the worst case we'll merge the ranges a bit differently). regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index 70109960e8..439e0b414c 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -2090,7 +2090,7 @@ brin_minmax_multi_distance_timetz(PG_FUNCTION_ARGS) TimeTzADT *ta = PG_GETARG_TIMETZADT_P(0); TimeTzADT *tb = PG_GETARG_TIMETZADT_P(1); - delta = tb->time - ta->time; + delta = (tb->time + tb->zone * USECS_PER_SEC) - (ta->time + ta->zone * USECS_PER_SEC); Assert(delta >= 0); @@ -2292,6 +2292,9 @@ brin_minmax_multi_distance_inet(PG_FUNCTION_ARGS) inet *ipa = PG_GETARG_INET_PP(0); inet *ipb = PG_GETARG_INET_PP(1); + int lena, + lenb; + /* * If the addresses are from different families, consider them to be in * maximal possible distance (which is 1.0). @@ -2299,20 +2302,38 @@ brin_minmax_multi_distance_inet(PG_FUNCTION_ARGS) if (ip_family(ipa) != ip_family(ipb)) PG_RETURN_FLOAT8(1.0); - /* ipv4 or ipv6 */ - if (ip_family(ipa) == PGSQL_AF_INET) - len = 4; - else - len = 16; /* NS_IN6ADDRSZ */ - addra = ip_addr(ipa); addrb = ip_addr(ipb); + /* + * The length is calculated from the mask length, because we sort the + * addresses by first address in the range, so A.B.C.D/24 < A.B.C.1 + * (the first range starts at A.B.C.0, which is before A.B.C.1). We + * don't want to produce negative delta in this case, so we just cut + * the extra bytes. + * + * XXX Maybe this should be a bit more careful and cut the bits, not + * just whole bytes. + */ + lena = ip_bits(ipa) / 8; + lenb = ip_bits(ipb) / 8; + + len = Max(lena, lenb); + delta = 0; for (i = len - 1; i >= 0; i--) { - delta += (float8) addrb[i] - (float8) addra[i]; - delta /= 256; + unsigned char a = addra[i]; + unsigned char b = addrb[i]; + + if (i >= lena) + a = 0; + + if (i >= lenb) + b = 0; + + delta += (float8) b - (float8) a; + delta /= 255; } Assert((delta >= 0) && (delta <= 1));