Thread: small exclusion constraints patch

small exclusion constraints patch

From
Jeff Davis
Date:
Currently, the check for exclusion constraints performs a sanity check
that's slightly too strict -- it assumes that a tuple will conflict with
itself. That is not always the case: the operator might be "<>", in
which case it's perfectly valid for the search for conflicts to not find
itself.

This patch simply removes that sanity check, and leaves a comment in
place.

Regards,
    Jeff Davis

Attachment

Re: small exclusion constraints patch

From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes:
> Currently, the check for exclusion constraints performs a sanity check
> that's slightly too strict -- it assumes that a tuple will conflict with
> itself. That is not always the case: the operator might be "<>", in
> which case it's perfectly valid for the search for conflicts to not find
> itself.

> This patch simply removes that sanity check, and leaves a comment in
> place.

I'm a bit uncomfortable with removing the sanity check; it seems like a
good thing to have, especially since this code hasn't even made it out
of beta yet.  AFAIK the "<>" case is purely hypothetical, because we
have no index opclasses supporting such an operator, no?  How about just
documenting that we'd need to remove the sanity check if we ever did add
support for such a case?
        regards, tom lane


Re: small exclusion constraints patch

From
Bruce Momjian
Date:
Tom Lane wrote:
> Jeff Davis <pgsql@j-davis.com> writes:
> > Currently, the check for exclusion constraints performs a sanity check
> > that's slightly too strict -- it assumes that a tuple will conflict with
> > itself. That is not always the case: the operator might be "<>", in
> > which case it's perfectly valid for the search for conflicts to not find
> > itself.
>
> > This patch simply removes that sanity check, and leaves a comment in
> > place.
>
> I'm a bit uncomfortable with removing the sanity check; it seems like a
> good thing to have, especially since this code hasn't even made it out
> of beta yet.  AFAIK the "<>" case is purely hypothetical, because we
> have no index opclasses supporting such an operator, no?  How about just
> documenting that we'd need to remove the sanity check if we ever did add
> support for such a case?

Done, with attached, applied patch.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
Index: src/backend/executor/execUtils.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execUtils.c,v
retrieving revision 1.171
diff -c -c -r1.171 execUtils.c
*** src/backend/executor/execUtils.c    26 Feb 2010 02:00:41 -0000    1.171
--- src/backend/executor/execUtils.c    29 May 2010 02:30:23 -0000
***************
*** 1310,1316 ****

      /*
       * We should have found our tuple in the index, unless we exited the loop
!      * early because of conflict.  Complain if not.
       */
      if (!found_self && !conflict)
          ereport(ERROR,
--- 1310,1317 ----

      /*
       * We should have found our tuple in the index, unless we exited the loop
!      * early because of conflict.  Complain if not.  If we ever implement
!      * '<>' index opclasses, this check will fail and will have to be removed.
       */
      if (!found_self && !conflict)
          ereport(ERROR,

Re: small exclusion constraints patch

From
Robert Haas
Date:
On Fri, May 28, 2010 at 10:32 PM, Bruce Momjian <bruce@momjian.us> wrote:
> Tom Lane wrote:
>> Jeff Davis <pgsql@j-davis.com> writes:
>> > Currently, the check for exclusion constraints performs a sanity check
>> > that's slightly too strict -- it assumes that a tuple will conflict with
>> > itself. That is not always the case: the operator might be "<>", in
>> > which case it's perfectly valid for the search for conflicts to not find
>> > itself.
>>
>> > This patch simply removes that sanity check, and leaves a comment in
>> > place.
>>
>> I'm a bit uncomfortable with removing the sanity check; it seems like a
>> good thing to have, especially since this code hasn't even made it out
>> of beta yet.  AFAIK the "<>" case is purely hypothetical, because we
>> have no index opclasses supporting such an operator, no?  How about just
>> documenting that we'd need to remove the sanity check if we ever did add
>> support for such a case?
>
> Done, with attached, applied patch.

The only disadvantage I see of just documenting this is that someone
might write a user-defined index opclass that works like this, and
they won't be able to use this until at least 9.1 (or at least, not
without patching the source).

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


Re: small exclusion constraints patch

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> The only disadvantage I see of just documenting this is that someone
> might write a user-defined index opclass that works like this, and
> they won't be able to use this until at least 9.1 (or at least, not
> without patching the source).

I don't actually think that anyone's very likely to write a <>-like index
operator.  It's approximately useless to use an index for such a query.

Or, to put it differently: if nobody's done that in the past twenty
years, why is it likely to happen before 9.1?
        regards, tom lane


Re: small exclusion constraints patch

From
David Fetter
Date:
On Sat, May 29, 2010 at 06:11:57PM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > The only disadvantage I see of just documenting this is that
> > someone might write a user-defined index opclass that works like
> > this, and they won't be able to use this until at least 9.1 (or at
> > least, not without patching the source).
> 
> I don't actually think that anyone's very likely to write a <>-like
> index operator.  It's approximately useless to use an index for such
> a query.
> 
> Or, to put it differently: if nobody's done that in the past twenty
> years, why is it likely to happen before 9.1?

Because there's a fundamentally new way to use them now, namely with
exclusion constraints :)

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


Re: small exclusion constraints patch

From
Robert Haas
Date:
On Sat, May 29, 2010 at 6:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> The only disadvantage I see of just documenting this is that someone
>> might write a user-defined index opclass that works like this, and
>> they won't be able to use this until at least 9.1 (or at least, not
>> without patching the source).
>
> I don't actually think that anyone's very likely to write a <>-like index
> operator.  It's approximately useless to use an index for such a query.
>
> Or, to put it differently: if nobody's done that in the past twenty
> years, why is it likely to happen before 9.1?

Hmm.  Well suppose we bet a dollar on whether that will happen or not.In fact, if you promise not to read
http://archives.postgresql.org/pgsql-hackers/2010-05/msg01175.php I'll
make it two dollars.

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


Re: small exclusion constraints patch

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Sat, May 29, 2010 at 6:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Or, to put it differently: if nobody's done that in the past twenty
>> years, why is it likely to happen before 9.1?

> Hmm.  Well suppose we bet a dollar on whether that will happen or not.
>  In fact, if you promise not to read
> http://archives.postgresql.org/pgsql-hackers/2010-05/msg01175.php I'll
> make it two dollars.

Yes, I've seen Jeff's example.  It's a cute hack but somehow I doubt
that there is going to be a land rush to implement such things.
Can you point to any pre-existing example where anyone actually asked
for the ability to do that?
        regards, tom lane


Re: small exclusion constraints patch

From
Robert Haas
Date:
On Sat, May 29, 2010 at 11:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sat, May 29, 2010 at 6:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Or, to put it differently: if nobody's done that in the past twenty
>>> years, why is it likely to happen before 9.1?
>
>> Hmm.  Well suppose we bet a dollar on whether that will happen or not.
>>  In fact, if you promise not to read
>> http://archives.postgresql.org/pgsql-hackers/2010-05/msg01175.php I'll
>> make it two dollars.
>
> Yes, I've seen Jeff's example.  It's a cute hack but somehow I doubt
> that there is going to be a land rush to implement such things.
> Can you point to any pre-existing example where anyone actually asked
> for the ability to do that?

I've often wished for the ability to constrain a tale to hold just one
row, so I don't find that use case implausible at all.  As to whether
Jeff's use case is a cute hack or something that people will really
want to do, I think the jury's still out on that one.  But I don't
think we should make it not work unless we have a concrete reason, and
I haven't heard one yet.

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


Re: small exclusion constraints patch

From
Marko Tiikkaja
Date:
On 2010-05-30 06:55 +0300, Robert Haas wrote:
> On Sat, May 29, 2010 at 11:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yes, I've seen Jeff's example.  It's a cute hack but somehow I doubt
>> that there is going to be a land rush to implement such things.
>> Can you point to any pre-existing example where anyone actually asked
>> for the ability to do that?
> 
> I've often wished for the ability to constrain a tale to hold just one
> row, so I don't find that use case implausible at all.

As I pointed out in
http://archives.postgresql.org/pgsql-hackers/2010-05/msg01177.php , you
can already do that.

Having said that, I also think that supporting <> in exclusion
constraints would be useful.  I can't come up with a real-world use case
right now though.


Regards,
Marko Tiikkaja


Re: small exclusion constraints patch

From
Tom Lane
Date:
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
> On 2010-05-30 06:55 +0300, Robert Haas wrote:
>> I've often wished for the ability to constrain a tale to hold just one
>> row, so I don't find that use case implausible at all.

> As I pointed out in
> http://archives.postgresql.org/pgsql-hackers/2010-05/msg01177.php , you
> can already do that.

Yes.  This is NOT about constraining a table to hold only one row.
It's about requiring all its rows to hold the same value (in some
column(s)), without predetermining exactly which value that will be.
I think the use-case for that is really extremely narrow.
        regards, tom lane


Re: small exclusion constraints patch

From
Robert Haas
Date:
On Sun, May 30, 2010 at 10:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
>> On 2010-05-30 06:55 +0300, Robert Haas wrote:
>>> I've often wished for the ability to constrain a tale to hold just one
>>> row, so I don't find that use case implausible at all.
>
>> As I pointed out in
>> http://archives.postgresql.org/pgsql-hackers/2010-05/msg01177.php , you
>> can already do that.
>
> Yes.  This is NOT about constraining a table to hold only one row.
> It's about requiring all its rows to hold the same value (in some
> column(s)), without predetermining exactly which value that will be.
> I think the use-case for that is really extremely narrow.

It probably is pretty narrow.  After all, exclusion constraints in
general are something that not everyone needs, and the ordinary use
case of preventing two intervals or regions from overlapping will
doubtless be far more common than this one.  Still, I'm not sure how
that's relevant.  The fact that not very many people will want to do
something is not a reason to prevent it.

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


Re: small exclusion constraints patch

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> ... The fact that not very many people will want to do
> something is not a reason to prevent it.

It's not about preventing it for no reason.  The proposed patch removes a
significant sanity check from code that still hasn't gotten out of beta.
I might be willing to remove it in 9.1; I am *not* willing to remove it
from 9.0.
        regards, tom lane


Re: small exclusion constraints patch

From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes:
> Currently, the check for exclusion constraints performs a sanity check
> that's slightly too strict -- it assumes that a tuple will conflict with
> itself. That is not always the case: the operator might be "<>", in
> which case it's perfectly valid for the search for conflicts to not find
> itself.

> This patch simply removes that sanity check, and leaves a comment in
> place.

Applied (for 9.1 only).
        regards, tom lane