Re: [PATCH] Support empty ranges with bounds information

On Tue, Mar 2, 2021, at 21:40, Mark Dilger wrote:
>
>
> > On Mar 2, 2021, at 12:04 PM, Joel Jacobson <joel(at)compiler(dot)org> wrote:
> >
> > On Tue, Mar 2, 2021, at 20:57, Mark Dilger wrote:
> >> I didn't phrase that clearly enough. I'm thinking about whether you include the bounds information in the hash function. The current implementation of hash_range(PG_FUNCTION_ARGS) is going to hash the lower and upper bounds, since you didn't change it to do otherwise, so "equal" values won't always hash the same. I haven't tested this out, but it seems you could get a different set of rows depending on whether the planner selects a hash join.
> >
> > I think this issue is solved by the empty-ranges-with-bounds-information-v2.patch I just sent,
> > since with it, there are no semantic changes at all, lower() and upper() works like before.
>
> There are semantic differences, because hash_range() doesn't call lower() and upper(), it uses RANGE_HAS_LBOUND and RANGE_HAS_UBOUND, the behavior of which you have changed. I created a regression test and expected results and checked after applying your patch, and your patch breaks the hash function behavior. Notice that before your patch, all three ranges hashed to the same value, but not so after:
>
>
> @@ -1,18 +1,18 @@
> select hash_range('[a,a)'::textrange);
> hash_range
> ------------
> - 484847245
> + -590102690
> (1 row)
>
> select hash_range('[b,b)'::textrange);
> hash_range
> ------------
> - 484847245
> + 281562732
> (1 row)
>
> select hash_range('[c,c)'::textrange);
> - hash_range
> -------------
> - 484847245
> + hash_range
> +-------------
> + -1887445565
> (1 row)
>
>
> You might change how hash_range() works to get all "equal" values to hash the same value, but that just gets back to the problem that non-equal things appear to be equal. I guess that's your two-warty-feet preference, but not everyone is going to be in agreement on that.

Yikes. Here be dragons. I think I want my wart free foot back please.

Many thanks for explaining. I think I’ll abandon this patch. I guess implementing an entirely new range type could be an acceptable solution, but that’s too big of a project for me to manage on my own. If any more experienced hackers are interested in such a project, I would love to help if I can.

>
> —
> Mark Dilger
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
>
>

Kind regards,

Joel