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

Reply via email to