Re: CREATE IF NOT EXISTS INDEX

On Wed, Oct 1, 2014 at 2:42 PM, Fabrízio de Royes Mello
<fabriziomello(at)gmail(dot)com> wrote:
> So, what's the correct/best grammar?
> CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX index_name
> or
> CREATE [ UNIQUE ] INDEX [ IF NOT EXISTS ] index_name

I've elected myself as the reviewer for this patch. Here are some
preliminary comments...

I agree with José. The 2nd is more consistent given the other syntaxes:
CREATE { TABLE | SCHEMA | EXTENSION | ... } IF NOT EXISTS name ...
It's also compatible with SQLite's grammar:
https://www.sqlite.org/lang_createindex.html

Do we want to enforce an order on the keywords or allow both?
CREATE INDEX IF NOT EXISTS CONCURRENTLY foo ...
CREATE INDEX CONCURRENTLY IF NOT EXISTS foo ...

It's probably very rare to use both keywords at the same time, so I'd
prefer only the 2nd, unless someone else chimes in.

Documentation: I would prefer if the explanation were consistent with
the description for ALTER TABLE/EXTENSION; just copy it and replace
"relation" with "index".

+ ereport(NOTICE,
+ (errcode(ERRCODE_DUPLICATE_TABLE),
+ errmsg("relation \"%s\" already exists, skipping",
+ indexRelationName)));

1. Clearly "relation" should be "index".
2. Use ERRCODE_DUPLICATE_OBJECT not TABLE

+ if (n->if_not_exists && n->idxname == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("IF NOT EXISTS requires that you
name the index."),

I think ERRCODE_SYNTAX_ERROR makes more sense, it's something that we
decided we *don't want* to support.

- write_msg(NULL, "reading row-security enabled for table \"%s\"",
+ write_msg(NULL, "reading row-security enabled for table \"%s\"\n",

???

Regards,
Marti