Re: CREATE IF NOT EXISTS INDEX

On Tue, Oct 7, 2014 at 2:42 AM, Fabrízio de Royes Mello
<fabriziomello(at)gmail(dot)com> wrote:
> On Mon, Oct 6, 2014 at 11:13 AM, Marti Raudsepp <marti(at)juffo(dot)org> wrote:
>>
>> On Mon, Oct 6, 2014 at 4:27 PM, Fabrízio de Royes Mello
>> <fabriziomello(at)gmail(dot)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