Hello, > I'd like to put comments on 0001 and 0004 only now: ...
I don't have a comment on 0002. About 0003: > @@ -4487,21 +4486,21 @@ circle_in(PG_FUNCTION_ARGS) > ... > circle->radius = single_decode(s, &s, "circle", str); > - if (circle->radius < 0) > + if (float8_lt(circle->radius, 0.0)) > ereport(ERROR, > (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), flost8_lt and its family functions are provided to unify the sorting order including NaN. NaN is not rejected by the usage of float8_lt in the case but it is what the function is expected to be used for. If we wanted to check if it is positive, it unexpectedly throws an exception. (I suppose that NaNs should be silently ignored rather than stopping a query by throwng an exception.) Addition to that I don't think it proper that applying EPSILON(!) there. It should be strictly compared regardless whether EPSION is considered or not. Similary, circle_overlap for example, float8_le is used to compare the distance and the summed radius. NaN causes a problem in another place. > PG_RETURN_BOOL(FPle(point_dt(&circle1->center, &circle2->center), > float8_pl(circle1->radius, circle2->radius))); If the distance was NaN and the summed radius is not, the function returns true. I think that a reasonable behavior is that an object containing NaN cannot make any meaningful relationship with other objects as floating number itself behave so. (Any comparison other than != with NaN returns always false) Just using another series of comparison functions that return false for NaN-containing comparison is not a solution since the meaning of the returned false differs by context, just same as the first problem above. For exameple, the fictious functions below, | bool circle_overlaps() | ret = FPle(distance, radius_sum); This gives correct results, but | bool circle_not_overlaps() | ret = FPgt(distance, radius_sum); This gives a wrong result for NaN-containing objects. Perhaps it is enough to explicitly define behaviors for NaN before comparison. circle_overlap() > distance = point_dt(....); > radius_sum = float8_pl(...); > > /* NaN-containing objects doesn't overlap any other objects */ > if (isnan(distance) || isnan(radius_sum)) > PG_RETURN_BOOL(false); > > /* NaN ordering of FPle() doesn't get into mischief here */ > return PG_RETURN_BOOL(FPle(distance, radius_sum)); (End Of the Comment to 0003) regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers