I have not looked at the issue with the btree_gin tests yet, but here is
the first part of my review.
= Review
This is my first quick review where I just read the documentation and
quickly tested the feature. I will review it more in-depth later.
This is a very useful feature, one which I have a long time wished for.
The patch applies, compiles and passes the test suite with just one warning.
parse_coerce.c: In function ‘select_common_type_2args’:
parse_coerce.c:1379:7: warning: statement with no effect [-Wunused-value]
rightOid;
^~~~~~~~
= Functional
The documentation does not agree with the code on the syntax. The
documentation claims it is "FOREIGN KEY (ELEMENT xs) REFERENCES t1 (x)"
when it actually is "FOREIGN KEY (EACH ELEMENT OF xs) REFERENCES t1 (x)".
Likewise I can't get the "final_positions integer[] ELEMENT REFERENCES
drivers" syntax to work, but here I cannot see any change in the syntax
to support it.
Related to the above: I am not sure if it is a good idea to make ELEMENT
a reserved word in column definitions. What if the SQL standard wants to
use it for something?
The documentation claims ON CASCADE DELETE is not supported by array
element foreign keys, but I do not think that is actually the case.
I think I prefer (EACH ELEMENT OF xs) over (ELEMENT xs) given how the
former is more in what I feel is the spirit of SQL. And if so we should
match it as "xs integer[] EACH ELEMENT REFERENCES t1 (x)", assuming we
want that syntax.
Once I have created an array element foreign key the basic features seem
to work as expected.
The error message below fails to mention that it is an array element
foreign key, but I do not think that is not a blocker for getting this
feature merged. Right now I cannot think of how to improve it either.
$ INSERT INTO t3 VALUES ('{1,3}');
ERROR: insert or update on table "t3" violates foreign key constraint
"t3_xs_fkey"
DETAIL: Key (xs)=({1,3}) is not present in table "t1".
= Nitpicking/style comments
In doc/src/sgml/catalogs.sgml the
"<entry><structfield>conpfeqop</structfield></entry>" line is
incorrectly indented.
I am not fan of calling it "array-vs-scalar". What about array to scalar?
In ddl.sgml date should be lower case like the other types in "race_day
DATE,".
In ddl.sgml I suggest removing the "..." from the examples to make it
possible to copy paste them easily.
Your text wrapping in ddl.sqml and create_table.sgqml is quite
arbitrary. I suggest wrapping all paragraphs at 80 characters (except
for code which should not be wrapped). Your text editor probably has
tools for wrapping paragraphs.
Please be consistent about how you write table names and SQL in general.
I think almost all places use lower case for table names, while your
examples in create_table.sgml are FKTABLEFORARRAY.
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
