On Thu, Oct 2, 2014 at 8:15 PM, Marti Raudsepp <marti@juffo.org> wrote: > > On Wed, Oct 1, 2014 at 2:42 PM, Fabrízio de Royes Mello > <fabriziomello@gmail.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... >
Thanks...
> 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. >
Fixed.
> Documentation: I would prefer if the explanation were consistent with > the description for ALTER TABLE/EXTENSION; just copy it and replace > "relation" with "index". >
Well, I'm not native English so I tend to agree with you.
"Do not throw an error if the index already exists. A notice is issued in this case."
Fixed in that way. Ok?
> + 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 >
Sorry, but I'm not with you. I just copy the original error message and add ", skipping" at the end.
> + 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. >
I don't think so. It's the same as CREATE SCHEMA IF NOT EXISTS that not support to include schema elements.