2009/7/15 Jeff Davis <pg...@j-davis.com>: > Updated patch attached. > > Changes: > * Added syntax support: > CREATE INDEX foo_idx ON foo ... (a CONSTRAINT =, b CONSTRAINT &&); > * More aggressively clear the shared memory entries to avoid > unnecessary checks > * Code cleanup > > TODO: > * When adding constraint to table with data already in it, verify that > existing data satisfies constraint. > * Clean up error messages a little > * Docs
Hi Jeff, I've been assigned to do an initial review of your patch. Because this patch is still WIP, there's not a lot for me to do. I see from the thread that there are still some design questions that remain open, but I won't be addressing those. The internals of indexes and constraints is not an area of the code I have any particular insight about. The patch applies cleanly, builds successfully and passes regression tests. I read through the code changes, and didn't notice any code convention violations. I had a play around with the feature in psql. I think the syntax is okay, but using "ALTER TABLE ... ADD" as you mentioned upthread could be a better option. I noticed that there's no change to the output of \d in psql to show the constraint, so when I do a \d on my test table, I can see that there's a gist index there, but I can't tell that there is also a constraint on it. This seems like a pretty significant shortcoming. Essentially once you've created one of these index constraints, it vanishes into the catalogs and becomes invisible to the user. This might call for a modification of pg_get_indexdef()? You've already noted this as a TODO, but clearly the error messages need some serious help. If I deliberately violate an index constraint I get: ERROR: conflict detected 3 At minimum the message should use the term "constraint" and give the name of the index that has detected the conflict. I think something like this would be okay: ERROR: new record violates constraint on index "circle_idx" I also noticed that the patch does not include any changes or additions to the regression test suite. Perhaps it ought to? That's all the feedback I have for the moment. I hope you find my comments constructive. 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
