Re: CREATE IF NOT EXISTS INDEX

On Thu, Oct 30, 2014 at 12:11 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> 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.
>

Fixed.

> + 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.
>

Fixed.

> + Index name is required when IF NOT EXISTS is specified.
>
> IF NOT EXISTS should be enclosed with <literal> tag.
>

Fixed.

> @@ -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.
>

Fixed.

> + 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 "?".
>

Fixed.

Thanks for your review!

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello