Re: operator exclusion constraints

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Jeff Davis <pgsql(at)j-davis(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: operator exclusion constraints
Date: 2009-11-25 14:02:05
Message-ID: 603c8f070911250602y342cc3b7g19bb8cd42eae8a54@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 25, 2009 at 3:23 AM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>> I was thinking maybe you call BuildIndexValueDescription twice and
>> make the error message say something like <output of first call>
>> conflicts with <output of second call>.
>
> Do you really think that's a better error message, or are you just
> trying to re-use similar code?
>
> Let's start from how the error message should read, and then see if we
> can re-use some code to make it look that way. It's one of the most
> visible aspects of the feature, and it needs to be reasonably concise
> and understandable in the simple case, but contain all of the necessary
> information.
>
> I think it's better to avoid the "=" when describing the conflict. I
> tend to read it as "equals" even though it's just punctuation in this
> case, so it would be distracting. I could change it to a colon, I
> suppose.

I disagree wholeheartedly. :-) My ideal error message is something like:

DETAIL: (a, b, c)=(1, 2, 3) conflicts with (a, b, c)=(4, 5, 6)

In particular, I think it's very important that we only emit the
columns which are part of the operator exclusion constraints, and NOT
all the columns of the tuple. The entire tuple could be very large -
one of the columns not involved in the constraint could be a 4K block
of text, for example, and spitting that out only obscures the real
source of the problem. You could argue that one of the columns
involved in the constraint could be a 4K block text, too, but in
practice I think the constraint columns are likely to be short nine
times out of ten. The equals sign is equating the column names to the
column values, not the values to each other, so I don't see that as
confusing.

>> create table test (a int4[], exclude using gist (a with =));
>> ERROR:  operator does not exist: integer[] = integer[]
>
> Thanks, fixed. I am now using compatible_oper_opid(), which will find
> any operators that don't require binary-incompatible coercion of the
> operands.
>
> Do you think there's any reason to support binary-incompatible coercion
> of the operands? I can't think of a single use case, and if you really
> need to, you can coerce the types explicitly in the expression.

My operator-class-fu is insufficient to render judgment on this point.
I think the thing to do is look at a bunch of non-built-in opclasses
and check for POLA violations.

...Robert

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2009-11-25 14:05:44 Re: SE-PgSQL patch review
Previous Message Simon Riggs 2009-11-25 14:00:38 Hot Standby and cancelling idle queries