Re: operator exclusion constraints - Mailing list pgsql-hackers

On Fri, 2009-11-13 at 23:39 -0500, Robert Haas wrote:
> [ reviewing ]

Thank you for the comments so far.

> In index_create(), the elog() where relopxconstraints < 0 should just
> complain about the value being negative, I think, rather than listing
> the value.  If you just say the value is -3, it doesn't give the user
> a clue why that's bad.

Hopefully the user never sees that message -- it's almost an Assert.
PostgreSQL uses elog(ERROR,...) in many places that should be
unreachable, but might happen due to bugs in distant places or
corruption. I'm not sure the exact convention there, but I figure that
some details are appropriate.

I tried to make all user-visible errors into ereport()s.

> In ATAddOperatorExclusionConstraint(), the message "method %s does not
> support gettuple" seems a bit user-unfriendly.  Can we explain the
> problem by referring to the functionality of getttuple(), rather than
> the name of it?

How about "operator exclusion constraints don't support method X"? Then
perhaps have a detail-level message to explain that the access method
needs to support the gettuple interface.

Trying to describe what gettuple does doesn't help a humble user much.
All they care about is "can't use gin".

> I don't really like this message, for a number of reasons.
> 
> alter table foo add constraint bar exclude (a check with =, b check with =);
> ERROR:  operator exclusion constraint violation detected: "foo_a_exclusion"
> DETAIL:  Tuple "(1, 1, 2)" conflicts with existing tuple "(1, 1, 3)".
> 
> The corresponding error for a UNIQUE index is: could not create unique
> index "bar", which I like better.  Only the relevant columns from the
> tuples are dumped, and the tuple is not surrounded by double quotes;
> any reason not to parallel that here?

By "relevant columns" I assume you mean the entire index tuple. That
means we need to have column names represented somewhere, because we
don't want the user to have to match up ordinal index columns.

Also, with exclusion constraints, both values are always relevant, not
just the one being inserted. What if the user just wants to adjust their
request slightly to avoid an overlap -- how do they know how far to go?
I know this could be accomplished with extra queries, as well, but that
doesn't always work for someone looking through the logs after the fact,
when the original values may be gone.

So, the kind of error message you're suggesting starts to get awkward: (a: 1 = 1, b: 1 = 1)

or something? And then with more complex type output functions, and
expression indexes, it starts to look very complex.

What do you think is the cleanest approach?

> Also, the message is all
> lower-case.

I know the error conventions are documented somewhere, but I completely
forgot where. Can you please point me to the right place? I thought most
error messages were supposed to be lower case, and detail messages were
supposed to read like sentences.

> Similarly, for an insert/update situation, it seems that
> a message like "key value violates exclusion constraint \"%s\"" would
> be better than the existing message.

I can certainly simplify it, but I was trying to match the usefulness of
UNIQUE constraint error messages.

> As a quick performance test, I inserted a million 3-integer tuples
> into a 3-column table with a unique constraint or an operator
> exclusion constraint over all three columns.  The former took ~ 15 s,
> the latter ~ 21.5 s.  That seems acceptable.

Great news. I had similar results, though they depend on the conflict
percentage as well (I assume you had zero conflicts).

Regards,Jeff Davis