Re: Bogus documentation for bogus geometric operators

From: Emre Hasegeli <emre(at)hasegeli(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Bogus documentation for bogus geometric operators
Date: 2020-08-21 11:00:45
Message-ID: CAE2gYzzFb6kPJn1qTewPbUnq9O7Rn_4A+KcwD0npk3dht4_cvA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> While revising the docs for the geometric operators, I came across
> these entries:
>
> <^ Is below (allows touching)? circle '((0,0),1)' <^ circle '((0,5),1)'
> >^ Is above (allows touching)? circle '((0,5),1)' >^ circle '((0,0),1)'
>
> These have got more than a few problems:
>
> 1. There are no such operators for circles, so the examples are pure
> fantasy.
>
> 2. What these operators do exist for is points (point_below, point_above
> respectively) and boxes (box_below_eq, box_above_eq). However, the
> stated behavior is not what the point functions actually do:
>
> point_below(PG_FUNCTION_ARGS)
> ...
> PG_RETURN_BOOL(FPlt(pt1->y, pt2->y));
>
> point_above(PG_FUNCTION_ARGS)
> ...
> PG_RETURN_BOOL(FPgt(pt1->y, pt2->y));
>
> So point_below would be more accurately described as "is strictly below",
> so its operator name really ought to be <<|. And point_above is "is
> strictly above", so its operator name ought to be |>>.

I prepared a patch to add <<| and |>> operators for points to
deprecate the previous ones.

> 3. The box functions do seem to be correctly documented:
>
> box_below_eq(PG_FUNCTION_ARGS)
> ...
> PG_RETURN_BOOL(FPle(box1->high.y, box2->low.y));
>
> box_above_eq(PG_FUNCTION_ARGS)
> ...
> PG_RETURN_BOOL(FPge(box1->low.y, box2->high.y));
>
> But there are comments in the source code to the effect of
>
> * box_below_eq and box_above_eq are obsolete versions that (probably
> * erroneously) accept the equal-boundaries case. Since these are not
> * in sync with the box_left and box_right code, they are deprecated and
> * not supported in the PG 8.1 rtree operator class extension.
>
> I'm not sure how seriously to take this deprecation comment, but it
> is true that box_below (<<|) and box_above (|>>) have analogs for
> other data types while these functions don't.

I think we should take this comment seriously and deprecate those
operators, so the patch removes them from the documentation.

> 4. Just for extra fun, these point operators are listed in some
> GIST and SP-GIST opclasses; though the box ones are not, as per
> that code comment.

It also updates the operator classes to support the new operators
instead of former ones. I don't think there are many users of them to
notice the change.

I am adding this to the next commitfest.

Attachment Content-Type Size
0001-Add-and-operators-for-points-v01.patch application/octet-stream 38.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2020-08-21 11:39:46 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Previous Message John Naylor 2020-08-21 10:09:59 Re: Problems with the FSM, heap fillfactor, and temporal locality