On Thu, 2009-11-19 at 22:55 -0500, Robert Haas wrote:
> > At constraint definition time, I need to make sure that the strategy
> > numbers can be identified anyway, so it wouldn't save any work in
> > ATAddOperatorExclusionConstraint. At the time it seemed slightly more
> > direct to use strategy numbers in index_check_constraint, but it's
> > probably about the same.
>
> It sets off a red flag for me any time I see code that asks for A from
> the user and then actually stores B under the hood, for fear that the
> relationship that A and B might change. However...
Still not addressed because it touches a bit more code (including the
catalogs). I basically agree, however, so I'll take care of this soon.
> 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.
> 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.
Changes in attached patch:
* use compatible_oper_opid() to find operator from name
* Add new SQLSTATE 23P01 for the operator exclusion constraint error,
so that applications can more easily distinguish it from other
constraint violations.
Regards,
Jeff Davis