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