The patch looks good to me now other than some smaller issues, mostly in
the documentation. If you fix these I will set it as ready for
committer, and let a committer chime in on the casting logic which we
both find a bit ugly.
== Comments
The only a bit bigger issue I see is the error messages. Can they be
improved?
For example:
CREATE TABLE t1 (a int, b int, PrIMARY KEY (a, b));
CREATE TABLE t2 (a int, bs int8[], FOREIGN KEY (a, EACH ELEMENT OF bs)
REFERENCES t1 (a, b));
INSERT INTO t2 VALUES (0, ARRAY[1, 2]);
Results in:
ERROR: insert or update on table "t2" violates foreign key constraint
"t2_a_fkey"
DETAIL: Key (a, bs)=(0, {1,2}) is not present in table "t1".
Would it be possible to make the DETAIL something like the below
instead? Do you think my suggested error message is clear?
I am imaging something like the below as a patch. Does it look sane? The
test cases need to be updated at least.
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index 402bde19d4..9dc7c9812c 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -3041,6 +3041,10 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
appendStringInfoString(&key_names, ", ");
appendStringInfoString(&key_values, ", ");
}
+
+ if (riinfo->fk_reftypes[idx] == FKCONSTR_REF_EACH_ELEMENT)
+ appendStringInfoString(&key_names, "EACH ELEMENT OF ");
+
appendStringInfoString(&key_names, name);
appendStringInfoString(&key_values, val);
}
DETAIL: Key (a, EACH ELEMENT OF bs)=(0, {1,2}) is not present in table
"t1".
- REFERENCES <replaceable class="parameter">reftable</replaceable> [ (
<replaceable class="parameter">refcolumn</replaceable> ) ] [ MATCH FULL
| MATCH PARTIAL | MATCH SIMPLE ]
+ [EACH ELEMENT OF] REFERENCES <replaceable
class="parameter">reftable</replaceable> [ ( <replaceable
class="parameter">refcolumn</replaceable> ) ] [ MATCH FULL | MATCH
PARTIAL | MATCH SIMPLE ]
I think this documentation in doc/src/sgml/ref/create_table.sgml should
be removed since it is no longer correct.
+ <para>
+ Foreign Key Arrays are an extension
+ of PostgreSQL and are not included in the SQL standard.
+ </para>
This pargraph and some of the others you added to
doc/src/sgml/ref/create_table.sgml are strangely line wrapped.
+ <varlistentry>
+ <term><literal>ON DELETE CASCADE</literal></term>
+ <listitem>
+ <para>
+ Same as standard foreign key constraints. Deletes the entire
array.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
I thought this was no longer supported.
+ however, this syntax will cast ftest1 to int4 upon RI checks,
thus defeating the
+ purpose of the index.
There is a minor indentation error on these lines.
+
+ <para>
+ Array <literal>ELEMENT</literal> foreign keys and the <literal>ELEMENT
+ REFERENCES</literal> clause are a
<productname>PostgreSQL</productname>
+ extension.
+ </para>
We do not have any ELEMENT REFERENCES clause.
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("foreign key constraint \"%s\" "
- "cannot be implemented",
- fkconstraint->conname),
- errdetail("Key columns \"%s\" and \"%s\" "
- "are of incompatible types: %s and %s.",
- strVal(list_nth(fkconstraint->fk_attrs, i)),
- strVal(list_nth(fkconstraint->pk_attrs, i)),
- format_type_be(fktype),
- format_type_be(pktype))));
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("foreign key constraint \"%s\" "
+ "cannot be implemented",
+ fkconstraint->conname),
+ errdetail("Key columns \"%s\" and \"%s\" "
+ "are of incompatible types: %s and %s.",
+ strVal(list_nth(fkconstraint->fk_attrs, i)),
+ strVal(list_nth(fkconstraint->pk_attrs, i)),
+ format_type_be(fktype),
+ format_type_be(pktype))));
It seems like you accidentally change the indentation here,
Andreas
