Re: BUG #19603: Vuln47: distance_taxicab and distance_chebyshev silently return 0 instead of NaN when a cube coordin

Hi,

On Thu, 5 Nov 2026 at 13:47, Aleksander Alekseev
<aleksander(at)tigerdata(dot)com> wrote:
>
> Hi Ayush,
>
> Thanks for the quick update.

Thanks a lot for your review.

> > Thoughts on v2?
>
> v2 is much better but there is still some room for improvement:
>
> ```
> =# SELECT '(nan)'::cube AS a,
> cube(ARRAY['nan'::float8], ARRAY['nan'::float8]) AS b;
> a | b
> -------+-------------
> (NaN) | (NaN),(NaN)
> (1 row)
>
> =# SELECT '(nan)'::cube = cube(ARRAY['nan'::float8],
> ARRAY['nan'::float8]);
> ?column?
> ----------
> t <-- so they are "the same value"
>
> =# SELECT cube_is_point('(nan)'::cube),
> cube_is_point(cube(ARRAY['nan'::float8],
> ARRAY['nan'::float8]));
> cube_is_point | cube_is_point
> ---------------+---------------
> t | f <-- but one is a point and one is not

Ah, my bad. I took the comment in cubeparse.y as a reason to leave
those checks alone, but they need to agree with the comparisons.
Fixed in v3.

>
> =# SELECT cube_size('(nan)'::cube),
> cube_size(cube(ARRAY['nan'::float8],
> ARRAY['nan'::float8]));
> cube_size | cube_size
> -----------+-----------
> 0 | NaN
> ```
>
> This can be refactored as distance = float8_max(distance, d):

Yup, that's simpler. Changed both loops, thanks.

> ```
> @@ -1377,7 +1375,7 @@ distance_chebyshev(PG_FUNCTION_ARGS)
> {
> d = fabs(distance_1D(LL_COORD(a, i), UR_COORD(a, i),
> LL_COORD(b,
> i), UR_COORD(b, i)));
> - if (d > distance)
> + if (isnan(d) || d > distance)
> distance = d;
> }
>
> @@ -1385,7 +1383,7 @@ distance_chebyshev(PG_FUNCTION_ARGS)
> for (i = DIM(b); i < DIM(a); i++)
> {
> d = fabs(distance_1D(LL_COORD(a, i), UR_COORD(a, i), 0.0, 0.0));
> - if (d > distance)
> + if (isnan(d) || d > distance)
> distance = d;
> }
> ```
>
> The tests still pass if I remove:
>
> ```
> @@ -1503,6 +1503,11 @@ g_cube_distance(PG_FUNCTION_ARGS)
> break;
> }
> }
> +
> + /* NaN does not bound the subtree's distances from below */
> + if (!GIST_LEAF(entry) && isnan(retval))
> + retval = -get_float8_infinity();
> +
> PG_RETURN_FLOAT8(retval);
> }
> ```
>
> Which means the patch doesn't have a proper test case for this.

Thanks, I've changed the test in v3. I just wanted not to overboard
the test suite hence had left a few things out.

v3 attached.

Regards,
Ayush