2009/9/15 Jeff Davis <pg...@j-davis.com>:
> Attached is the latest version.

Hi Jeff,

I'm just getting started reviewing this version now.  I noticed that
your patch seems to have been generated by git.  Are you hosting this
work on a public repo somewhere that I can pull from?  Also I think
the committers generally prefer context diffs (pipe it through
"filterdiff --format=context --strip=1") in submissions.

Regarding the documentation updates, I think you might want to add
some commentary to Chapter 11: Indexes -- perhaps add a new section
after 11.6 Unique Indexes to talk about general index constraints,
and/or update the wording of 11.6 to reflect your changes.

The paragraph explaining index_constraints in ALTER TABLE may be a
little bit confusing.

<quote>
ADD index_constraint

    This form adds a new index constraint to the table which is
enforced by the given index. The operator provided must be usable as a
search strategy for the index, and it also must detect conflicts
symmetrically. The semantics are similar to a unique index but it
opens up new possibilities. A unique index can only detect conflicts
when the two values are equal, and that behavior is equivalent to an
index constraint where all operators are the equality operator.

    However, an index constraint allows you to use other operators as
well, such as the overlaps operator provided for the circle data type.
The index constraint will ensure that no two circles overlap. See
example below.
</quote>

My eyes started to cross in the second sentence.  "Detect conflicts
symmetrically"?  I have actually *used* this feature successfully in
testing the patch, and I still don't know quite what to make of that
phrase.  You might need to dumb it down.

It might also be good to be a bit more explicit about the way the
choice of operators works.  It is the inverse of the logic used to
express an ordinary value constraint.  E.g., when you use the equality
operator in an index constraint you are in effect saying that new rows
MUST NOT satisfy this operator for any existing rows.

I'll continue reviewing and post more comments on the code itself shortly.

Cheers,
BJ

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to