On Thu, Oct 2, 2014 at 8:15 PM, Marti Raudsepp <[email protected]> wrote:
>
> On Wed, Oct 1, 2014 at 2:42 PM, Fabrízio de Royes Mello
> <[email protected]> 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.
788 ereport(ERROR,
789 (errcode(ERRCODE_DUPLICATE_TABLE),
790 errmsg("relation \"%s\" already exists",
791 indexRelationName)));
> + 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.
Other opinions?
> - write_msg(NULL, "reading row-security enabled for table \"%s\"",
> + write_msg(NULL, "reading row-security enabled for table \"%s\"\n",
>
> ???
>
Sorry, my mistake. I merged with a wrong local branch. Fixed.
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
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index e469b17..06c2567 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</replaceable> ] ON <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">method</replaceable> ]
+CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</replaceable> ] ON <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">method</replaceable> ]
( { <replaceable class="parameter">column_name</replaceable> | ( <replaceable class="parameter">expression</replaceable> ) } [ COLLATE <replaceable class="parameter">collation</replaceable> ] [ <replaceable class="parameter">opclass</replaceable> ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [, ...] )
[ WITH ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] ) ]
[ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ]
@@ -99,6 +99,16 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
<variablelist>
<varlistentry>
+ <term><literal>IF NOT EXISTS</literal></term>
+ <listitem>
+ <para>
+ Do not throw an error if the index already exists. A notice is issued
+ in this case.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>UNIQUE</literal></term>
<listitem>
<para>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index ee10594..8905e30 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -697,7 +697,8 @@ index_create(Relation heapRelation,
bool allow_system_table_mods,
bool skip_build,
bool concurrent,
- bool is_internal)
+ bool is_internal,
+ bool if_not_exists)
{
Oid heapRelationId = RelationGetRelid(heapRelation);
Relation pg_class;
@@ -773,10 +774,22 @@ index_create(Relation heapRelation,
elog(ERROR, "shared relations must be placed in pg_global tablespace");
if (get_relname_relid(indexRelationName, namespaceId))
+ {
+ if (if_not_exists)
+ {
+ ereport(NOTICE,
+ (errcode(ERRCODE_DUPLICATE_TABLE),
+ errmsg("relation \"%s\" already exists, skipping",
+ indexRelationName)));
+ heap_close(pg_class, RowExclusiveLock);
+ return InvalidOid;
+ }
+
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_TABLE),
errmsg("relation \"%s\" already exists",
indexRelationName)));
+ }
/*
* construct tuple descriptor for index tuples
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 160f006..5ef6dcc 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -342,7 +342,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
rel->rd_rel->reltablespace,
collationObjectId, classObjectId, coloptions, (Datum) 0,
true, false, false, false,
- true, false, false, true);
+ true, false, false, true, false);
heap_close(toast_rel, NoLock);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8a1cb4b..a03773b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -610,7 +610,14 @@ DefineIndex(Oid relationId,
stmt->isconstraint, stmt->deferrable, stmt->initdeferred,
allowSystemTableMods,
skip_build || stmt->concurrent,
- stmt->concurrent, !check_rights);
+ stmt->concurrent, !check_rights,
+ stmt->if_not_exists);
+
+ if (!OidIsValid(indexRelationId))
+ {
+ heap_close(rel, NoLock);
+ return indexRelationId;
+ }
/* Add any requested comment */
if (stmt->idxcomment != NULL)
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 225756c..39b55db 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2907,6 +2907,7 @@ _copyIndexStmt(const IndexStmt *from)
COPY_SCALAR_FIELD(deferrable);
COPY_SCALAR_FIELD(initdeferred);
COPY_SCALAR_FIELD(concurrent);
+ COPY_SCALAR_FIELD(if_not_exists);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 905468e..4cf91e0 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1210,6 +1210,7 @@ _equalIndexStmt(const IndexStmt *a, const IndexStmt *b)
COMPARE_SCALAR_FIELD(deferrable);
COMPARE_SCALAR_FIELD(initdeferred);
COMPARE_SCALAR_FIELD(concurrent);
+ COMPARE_SCALAR_FIELD(if_not_exists);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 77d2f29..47b0a14 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6434,6 +6434,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_index_name
IndexStmt *n = makeNode(IndexStmt);
n->unique = $2;
n->concurrent = $4;
+ n->if_not_exists = false;
n->idxname = $5;
n->relation = $7;
n->accessMethod = $8;
@@ -6451,6 +6452,41 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_index_name
n->initdeferred = false;
$$ = (Node *)n;
}
+ | CREATE opt_unique INDEX opt_concurrently IF_P NOT EXISTS opt_index_name
+ ON qualified_name access_method_clause '(' index_params ')'
+ opt_reloptions OptTableSpace where_clause
+ {
+ IndexStmt *n = makeNode(IndexStmt);
+ n->unique = $2;
+ n->concurrent = $4;
+ n->if_not_exists = true;
+ n->idxname = $8;
+
+ /*
+ * Throw an exception when IF NOT EXISTS is used without an index name
+ */
+ if (n->idxname == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("IF NOT EXISTS requires that you name the index."),
+ parser_errposition(@5)));
+
+ n->relation = $10;
+ n->accessMethod = $11;
+ n->indexParams = $13;
+ n->options = $15;
+ n->tableSpace = $16;
+ n->whereClause = $17;
+ n->excludeOpNames = NIL;
+ n->idxcomment = NULL;
+ n->indexOid = InvalidOid;
+ n->oldNode = InvalidOid;
+ n->primary = false;
+ n->isconstraint = false;
+ n->deferrable = false;
+ n->initdeferred = false;
+ $$ = (Node *)n;
+ }
;
opt_unique:
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 12811a8..076ff8d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2803,7 +2803,7 @@ getRowSecurity(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading row-security enabled for table \"%s\"",
+ write_msg(NULL, "reading row-security enabled for table \"%s\"\n",
tbinfo->dobj.name);
/*
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 006b180..098ac7d 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -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);
extern void index_constraint_create(Relation heapRelation,
Oid indexRelationId,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f3aa69e..a326dc4 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2255,6 +2255,7 @@ typedef struct IndexStmt
bool deferrable; /* is the constraint DEFERRABLE? */
bool initdeferred; /* is the constraint INITIALLY DEFERRED? */
bool concurrent; /* should this be a concurrent index build? */
+ bool if_not_exists; /* just do nothing if index already exists */
} IndexStmt;
/* ----------------------
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index a2bef7a..ba60fbd 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -6,6 +6,12 @@
-- BTREE
--
CREATE INDEX onek_unique1 ON onek USING btree(unique1 int4_ops);
+CREATE INDEX IF NOT EXISTS onek_unique1 ON onek USING btree(unique1 int4_ops);
+NOTICE: relation "onek_unique1" already exists, skipping
+CREATE INDEX IF NOT EXISTS ON onek USING btree(unique1 int4_ops);
+ERROR: IF NOT EXISTS requires that you name the index.
+LINE 1: CREATE INDEX IF NOT EXISTS ON onek USING btree(unique1 int4_...
+ ^
CREATE INDEX onek_unique2 ON onek USING btree(unique2 int4_ops);
CREATE INDEX onek_hundred ON onek USING btree(hundred int4_ops);
CREATE INDEX onek_stringu1 ON onek USING btree(stringu1 name_ops);
@@ -2290,10 +2296,14 @@ create unique index hash_f8_index_3 on hash_f8_heap(random) where seqno > 1000;
CREATE TABLE concur_heap (f1 text, f2 text);
-- empty table
CREATE INDEX CONCURRENTLY concur_index1 ON concur_heap(f2,f1);
+CREATE INDEX CONCURRENTLY IF NOT EXISTS concur_index1 ON concur_heap(f2,f1);
+NOTICE: relation "concur_index1" already exists, skipping
INSERT INTO concur_heap VALUES ('a','b');
INSERT INTO concur_heap VALUES ('b','b');
-- unique index
CREATE UNIQUE INDEX CONCURRENTLY concur_index2 ON concur_heap(f1);
+CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS concur_index2 ON concur_heap(f1);
+NOTICE: relation "concur_index2" already exists, skipping
-- check if constraint is set up properly to be enforced
INSERT INTO concur_heap VALUES ('b','x');
ERROR: duplicate key value violates unique constraint "concur_index2"
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index d4d24ef..225727c 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -8,6 +8,10 @@
--
CREATE INDEX onek_unique1 ON onek USING btree(unique1 int4_ops);
+CREATE INDEX IF NOT EXISTS onek_unique1 ON onek USING btree(unique1 int4_ops);
+
+CREATE INDEX IF NOT EXISTS ON onek USING btree(unique1 int4_ops);
+
CREATE INDEX onek_unique2 ON onek USING btree(unique2 int4_ops);
CREATE INDEX onek_hundred ON onek USING btree(hundred int4_ops);
@@ -711,10 +715,12 @@ create unique index hash_f8_index_3 on hash_f8_heap(random) where seqno > 1000;
CREATE TABLE concur_heap (f1 text, f2 text);
-- empty table
CREATE INDEX CONCURRENTLY concur_index1 ON concur_heap(f2,f1);
+CREATE INDEX CONCURRENTLY IF NOT EXISTS concur_index1 ON concur_heap(f2,f1);
INSERT INTO concur_heap VALUES ('a','b');
INSERT INTO concur_heap VALUES ('b','b');
-- unique index
CREATE UNIQUE INDEX CONCURRENTLY concur_index2 ON concur_heap(f1);
+CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS concur_index2 ON concur_heap(f1);
-- check if constraint is set up properly to be enforced
INSERT INTO concur_heap VALUES ('b','x');
-- check if constraint is enforced properly at build time
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
