On Tue, Oct 7, 2014 at 2:42 AM, Fabrízio de Royes Mello <fabriziome...@gmail.com> wrote: > On Mon, Oct 6, 2014 at 11:13 AM, Marti Raudsepp <ma...@juffo.org> wrote: >> >> On Mon, Oct 6, 2014 at 4:27 PM, Fabrízio de Royes Mello >> <fabriziome...@gmail.com> wrote: >> > create_index_if_not_exists_v7.patch >> >> Looks good to me. Marking ready for committer. >> > > Thanks.
The patch looks good to me except the following minor comments. + <term><literal>IF NOT EXISTS</literal></term> It's better to place this after the paragraph of CONCURRENTLY for the consistency with the syntax. + Do not throw an error if the index already exists. I think that this should be Do not throw an error if a relation with the same name already exists. + Index name is required when IF NOT EXISTS is specified. IF NOT EXISTS should be enclosed with <literal> tag. @@ -60,7 +60,8 @@ extern Oid index_create(Relation heapRelation, bool allow_system_table_mods, bool skip_build, bool concurrent, - bool is_internal); + bool is_internal, + bool if_not_exists); You forgot to add the comment of if_not_exists argument into the top of index_create function. + bool if_not_exists; /* just do nothing if index already exists */ You forgot to add the trailing "?" at the above comment. There are similar comments in parsenodes.h, and they have such "?". Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers