Re: [PATCH] Add support function for containment operators

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Kim Johan Andersson <kimjand(at)kimmet(dot)dk>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Add support function for containment operators
Date: 2024-01-16 21:46:09
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

jian he <jian(dot)universality(at)gmail(dot)com> writes:
> [ v5-0001-Simplify-containment-in-range-constants-with-supp.patch ]

I spent some time reviewing and cleaning up this code. The major
problem I noted was that it doesn't spend any effort thinking about
cases where it'd be unsafe or undesirable to apply the transformation.
In particular, it's entirely uncool to produce a double-sided
comparison if the elemExpr is volatile. These two expressions
do not have the same result:

select random() <@ float8range(0.1, 0.2);
select random() >= 0.1 and random() < 0.2;

(Yes, I'm aware that BETWEEN is broken in this respect. All the
more reason why we mustn't break <@.)

Another problem is that even if the elemExpr isn't volatile,
it might be expensive enough that evaluating it twice is bad.
I am not sure where we ought to put the cutoff. There are some
existing places where we set a 10X cpu_operator_cost limit on
comparable transformations, so I stole that logic in the attached.
But perhaps someone has an argument for a different rule?

Anyway, pending discussion of that point, I think the code is good
to go. I don't like the test cases much though: they expend many more
cycles than necessary. You could prove the same points just by
looking at the expansion of expressions, eg.

regression=# explain (verbose, costs off) select current_date <@ daterange(null,null);
Output: true
(2 rows)

regression=# explain (verbose, costs off) select current_date <@ daterange('-Infinity', '1997-04-10'::date, '[)');
Output: ((CURRENT_DATE >= '-infinity'::date) AND (CURRENT_DATE < '1997-04-10'::date))
(2 rows)

I'd suggest losing the temp table and just coding tests like these.

regards, tom lane

Attachment Content-Type Size
v6-0001-Simplify-containment-in-range-constants-with-supp.patch text/x-diff 22.3 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Maiquel Grassi 2024-01-16 21:58:15 RE: New Window Function: ROW_NUMBER_DESC() OVER() ?
Previous Message Jim Nasby 2024-01-16 21:28:32 Re: Emit fewer vacuum records by reaping removable tuples during pruning