Thread: small exclusion constraints patch
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
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
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,
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
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
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
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
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
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
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
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
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
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
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