Fix BRIN minmax-multi distance for inet type

tvondra · tvondra · commit e1fbe1181c86 · 2021-04-04T19:23:32.000+02:00
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
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
@@ -2297,31 +2297,74 @@ 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).
 	 */
 	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);
 }
 
