Fix BRIN minmax-multi distance for inet type
authorTomas Vondra
Sun, 4 Apr 2021 17:23:30 +0000 (19:23 +0200)
committerTomas Vondra
Sun, 4 Apr 2021 17:23:32 +0000 (19:23 +0200)
The distance calculation ignored the mask, unlike the inet comparator,
which resulted in negative distance in some cases. Fixed by applying the
mask in brin_minmax_multi_distance_inet. I've considered simply calling
inetmi() to calculate the delta, but that does not consider mask either.

Reviewed-by: Zhihong Yu
Discussion: https://postgr.es/m/1a0a7b9d-9bda-e3a2-7fa4-88f15042a051%40enterprisedb.com

src/backend/access/brin/brin_minmax_multi.c

index e182cd95ecd007457213613e827b51845ef30a02..19ded7d933c9e52c91a395449fa63d499d8f3661 100644 (file)
@@ -2297,6 +2297,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).
@@ -2304,24 +2307,64 @@ 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 = (unsigned char *) palloc(ip_addrsize(ipa));
+   memcpy(addra, ip_addr(ipa), ip_addrsize(ipa));
+
+   addrb = (unsigned char *) palloc(ip_addrsize(ipb));
+   memcpy(addrb, ip_addr(ipb), ip_addrsize(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);
+   lenb = ip_bits(ipb);
+
+   len = ip_addrsize(ipa);
+
+   /* apply the network mask to both addresses */
+   for (i = 0; i < len; i++)
+   {
+       unsigned char   mask;
+       int             nbits;
 
-   addra = ip_addr(ipa);
-   addrb = ip_addr(ipb);
+       nbits = lena - (i * 8);
+       if (nbits < 8)
+       {
+           mask = (0xFF << (8 - nbits));
+           addra[i] = (addra[i] & mask);
+       }
 
+       nbits = lenb - (i * 8);
+       if (nbits < 8)
+       {
+           mask = (0xFF << (8 - nbits));
+           addrb[i] = (addrb[i] & mask);
+       }
+   }
+
+   /* Calculate the difference between the addresses. */
    delta = 0;
    for (i = len - 1; i >= 0; i--)
    {
-       delta += (float8) addrb[i] - (float8) addra[i];
+       unsigned char a = addra[i];
+       unsigned char b = addrb[i];
+
+       delta += (float8) b - (float8) a;
        delta /= 256;
    }
 
    Assert((delta >= 0) && (delta <= 1));
 
+   pfree(addra);
+   pfree(addrb);
+
    PG_RETURN_FLOAT8(delta);
 }