Re: (9.1) btree_gist support for searching on "not equals"

(1) Exclusion constraints support for operators where "x <operator> x"
is false (tiny patch)
https://commitfest.postgresql.org/action/patch_view?id=307
(2) btree_gist support for searching on <> ("not equals")
https://commitfest.postgresql.org/action/patch_view?id=308

Those patches should be committed at once because (2) requires (1) to work
with EXCLUDE constraints. Also, (1) has no benefits without (2) because we
have no use cases for <> as an index-able operator. Both patches are very
simple and small, and worked as expected both "WHERE <>" and EXCLUDE
constraints cases.

I'd like to ask you to write additional documentation about btree_gist [1]
that the module will be more useful when it is used with exclusion
constraints together. Without documentation, no users find the usages.
Of course the docs can be postponed if you have a plan to write docs
when PERIOD types are introduced,
[1] http://developer.postgresql.org/pgdocs/postgres/btree-gist.html

The patch was not applied to 9.0, but the reason was just "no time to test" [2].
We have enough time to test for 9.1, so we can apply it now!
[2] http://archives.postgresql.org/pgsql-hackers/2010-05/msg01874.php

--
Itagaki Takahiro

Hi,

Thank you for the review.

On Mon, 2010-07-12 at 17:17 +0900, Itagaki Takahiro wrote:
> (1) Exclusion constraints support for operators where "x <operator> x"
> is false (tiny patch)
> https://commitfest.postgresql.org/action/patch_view?id=307
> (2) btree_gist support for searching on <> ("not equals")
> https://commitfest.postgresql.org/action/patch_view?id=308
>
> Those patches should be committed at once because (2) requires (1) to work
> with EXCLUDE constraints. Also, (1) has no benefits without (2) because we
> have no use cases for <> as an index-able operator. Both patches are very
> simple and small, and worked as expected both "WHERE <>" and EXCLUDE
> constraints cases.

It appears that Tom already committed (1).

> I'd like to ask you to write additional documentation about btree_gist [1]
> that the module will be more useful when it is used with exclusion
> constraints together. Without documentation, no users find the usages.

Good idea, new patch attached.

Regards,
Jeff Davis

2010/7/16 Jeff Davis <pgsql(at)j-davis(dot)com>:
>> I'd like to ask you to write additional documentation about btree_gist [1]
>> that the module will be more useful when it is used with exclusion
>> constraints together. Without documentation, no users find the usages.

| Example using an Exclusion Constraint to enforce the constraint
| that a cage at a zoo can contain only one kind of animal:

Very interesting example :-)
The patch will be applied immediately.

--
Itagaki Takahiro

On Fri, Jul 16, 2010 at 1:19 AM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> Thank you for the review.
>
> On Mon, 2010-07-12 at 17:17 +0900, Itagaki Takahiro wrote:
>> (1) Exclusion constraints support for operators where "x <operator> x"
>> is false (tiny patch)
>> https://commitfest.postgresql.org/action/patch_view?id=307
>> (2) btree_gist support for searching on <> ("not equals")
>> https://commitfest.postgresql.org/action/patch_view?id=308
>>
>> Those patches should be committed at once because (2) requires (1) to work
>> with EXCLUDE constraints. Also, (1) has no benefits without (2) because we
>> have no use cases for <> as an index-able operator. Both patches are very
>> simple and small, and worked as expected both "WHERE <>" and EXCLUDE
>> constraints cases.
>
> It appears that Tom already committed (1).
>
>> I'd like to ask you to write additional documentation about btree_gist [1]
>> that the module will be more useful when it is used with exclusion
>> constraints together. Without documentation, no users find the usages.
>
> Good idea, new patch attached.

It seems pretty odd to define a constant called
BTNotEqualStrategyNumber in contrib/btree_gist. Shouldn't we either
call this something else, or define it in access/skey.h? Considering
that there seem to be some interesting gymnastics being done with
BTMaxStrategyNumber, I'd vote for the former. Maybe just
BtreeGistNotEqualStrategyNumber?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Sun, 2010-08-01 at 21:57 -0400, Robert Haas wrote:
> On Fri, Jul 16, 2010 at 1:19 AM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> > Thank you for the review.
> >
> > On Mon, 2010-07-12 at 17:17 +0900, Itagaki Takahiro wrote:
> >> (1) Exclusion constraints support for operators where "x <operator> x"
> >> is false (tiny patch)
> >> https://commitfest.postgresql.org/action/patch_view?id=307
> >> (2) btree_gist support for searching on <> ("not equals")
> >> https://commitfest.postgresql.org/action/patch_view?id=308
> >>
> >> Those patches should be committed at once because (2) requires (1) to work
> >> with EXCLUDE constraints. Also, (1) has no benefits without (2) because we
> >> have no use cases for <> as an index-able operator. Both patches are very
> >> simple and small, and worked as expected both "WHERE <>" and EXCLUDE
> >> constraints cases.
> >
> > It appears that Tom already committed (1).
> >
> >> I'd like to ask you to write additional documentation about btree_gist [1]
> >> that the module will be more useful when it is used with exclusion
> >> constraints together. Without documentation, no users find the usages.
> >
> > Good idea, new patch attached.
>
> It seems pretty odd to define a constant called
> BTNotEqualStrategyNumber in contrib/btree_gist. Shouldn't we either
> call this something else, or define it in access/skey.h? Considering
> that there seem to be some interesting gymnastics being done with
> BTMaxStrategyNumber, I'd vote for the former. Maybe just
> BtreeGistNotEqualStrategyNumber?

Sounds good to me.

At some point we may be interested to add this to BTree, as well. But we
can cross that bridge when we come to it.

Regards,
Jeff Davis

On Mon, Aug 2, 2010 at 2:39 AM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> On Sun, 2010-08-01 at 21:57 -0400, Robert Haas wrote:
>> On Fri, Jul 16, 2010 at 1:19 AM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>> > Thank you for the review.
>> >
>> > On Mon, 2010-07-12 at 17:17 +0900, Itagaki Takahiro wrote:
>> >> (1) Exclusion constraints support for operators where "x <operator> x"
>> >> is false (tiny patch)
>> >> https://commitfest.postgresql.org/action/patch_view?id=307
>> >> (2) btree_gist support for searching on <> ("not equals")
>> >> https://commitfest.postgresql.org/action/patch_view?id=308
>> >>
>> >> Those patches should be committed at once because (2) requires (1) to work
>> >> with EXCLUDE constraints. Also, (1) has no benefits without (2) because we
>> >> have no use cases for <> as an index-able operator. Both patches are very
>> >> simple and small, and worked as expected both "WHERE <>" and EXCLUDE
>> >> constraints cases.
>> >
>> > It appears that Tom already committed (1).
>> >
>> >> I'd like to ask you to write additional documentation about btree_gist [1]
>> >> that the module will be more useful when it is used with exclusion
>> >> constraints together. Without documentation, no users find the usages.
>> >
>> > Good idea, new patch attached.
>>
>> It seems pretty odd to define a constant called
>> BTNotEqualStrategyNumber in contrib/btree_gist.  Shouldn't we either
>> call this something else, or define it in access/skey.h?  Considering
>> that there seem to be some interesting gymnastics being done with
>> BTMaxStrategyNumber, I'd vote for the former.  Maybe just
>> BtreeGistNotEqualStrategyNumber?
>
> Sounds good to me.

OK, committed that way.

> At some point we may be interested to add this to BTree, as well. But we
> can cross that bridge when we come to it.

Yeah.

I was also wondering if it would be worth adding some additional
regression testing to contrib/btree_gist exercising this new
functionality. Thoughts?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Mon, 2010-08-02 at 12:27 -0400, Robert Haas wrote:
> I was also wondering if it would be worth adding some additional
> regression testing to contrib/btree_gist exercising this new
> functionality. Thoughts?

Sure. I attached two tests.

Regards,
Jeff Davis

On Mon, Aug 2, 2010 at 11:16 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> On Mon, 2010-08-02 at 12:27 -0400, Robert Haas wrote:
>> I was also wondering if it would be worth adding some additional
>> regression testing to contrib/btree_gist exercising this new
>> functionality.  Thoughts?
>
> Sure. I attached two tests.

Committed. I renamed the test to "not_equals" rather than "mixed" and
added an "EXPLAIN (COSTS OFF)" in there to verify that the index is
actually being used. (I might have to remove that if it turns out not
to be stable between an index scan and a bitmap index scan, but let's
see what the buildfarm says.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Aug 2, 2010 at 11:16 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>> Sure. I attached two tests.

> Committed.

I see no sign of a commit from here ...

regards, tom lane

On Tue, Aug 3, 2010 at 3:52 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Mon, Aug 2, 2010 at 11:16 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>>> Sure. I attached two tests.
>
>> Committed.
>
> I see no sign of a commit from here ...

Sigh. Forgot to exit my editor.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company