On Mon, Oct 27, 2014 at 4:15 AM, Rushabh Lathia <[email protected]>
wrote:
>
> Hi All,
>
> - Patch got applied cleanly.
> - Regression make check run fine.
> - Patch covered the documentation changes
>
> Here are few comments:
>
> 1) What the need of following change:
>
> diff --git a/src/backend/storage/lmgr/lwlock.c
b/src/backend/storage/lmgr/lwlock.c
> index bcec173..9fe6855 100644
> --- a/src/backend/storage/lmgr/lwlock.c
> +++ b/src/backend/storage/lmgr/lwlock.c
> @@ -1005,12 +1005,6 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr,
uint64 oldval, uint64 *newval)
> lock->tail = proc;
> lock->head = proc;
>
> - /*
> - * Set releaseOK, to make sure we get woken up as soon as the
lock is
> - * released.
> - */
> - lock->releaseOK = true;
> -
> /* Can release the mutex now */
> SpinLockRelease(&lock->mutex);
>
>
> It doesn't look like related to this patch.
>
Sorry... my mistake when diff to master (more updated than my branch).
Fixed.
> 2)
>
> diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
> index ae5fe88..4d11952 100644
> --- a/src/bin/psql/help.c
> +++ b/src/bin/psql/help.c
> @@ -247,7 +247,7 @@ slashUsage(unsigned short int pager)
> fprintf(output, _(" \\f [STRING] show or set field
separator for unaligned query output\n"));
> fprintf(output, _(" \\H toggle HTML output mode
(currently %s)\n"),
> ON(pset.popt.topt.format == PRINT_HTML));
> - fprintf(output, _(" \\pset [NAME [VALUE]] set table output
option\n"
> + fprintf(output, _(" \\pset [NAME [VALUE]] set table output
option\n"
> " (NAME :=
{format|border|expanded|fieldsep|fieldsep_zero|footer|null|\n"
> "
numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager|\n"
> "
unicode_border_linestyle|unicode_column_linestyle|unicode_header_linestyle})\n"));
>
>
> Why above changes reqired ?
>
Same previous mistake.
Fixed.
> 3) This patch adding IF NOT EXIST_S for CREATE TABLE AS and CREATE
MATERIALIZED
> TABLE, but testcase coverage for CREATE MATERIALIZED TABLE is missing.
>
> Apart from this changes looks good to me.
>
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
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 2c73852..8a0fb4d 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-CREATE MATERIALIZED VIEW <replaceable>table_name</replaceable>
+CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_name</replaceable>
[ (<replaceable>column_name</replaceable> [, ...] ) ]
[ WITH ( <replaceable class="PARAMETER">storage_parameter</replaceable> [= <replaceable class="PARAMETER">value</replaceable>] [, ... ] ) ]
[ TABLESPACE <replaceable class="PARAMETER">tablespace_name</replaceable> ]
@@ -53,6 +53,18 @@ CREATE MATERIALIZED VIEW <replaceable>table_name</replaceable>
<refsect1>
<title>Parameters</title>
+ <varlistentry>
+ <term><literal>IF NOT EXISTS</></term>
+ <listitem>
+ <para>
+ Do not throw an error if a materialized view with the same name already
+ exists. A notice is issued in this case. Note that there is no guarantee
+ that the existing materialized view is anything like the one that would
+ have been created.
+ </para>
+ </listitem>
+ </varlistentry>
+
<variablelist>
<varlistentry>
<term><replaceable>table_name</replaceable></term>
diff --git a/doc/src/sgml/ref/create_table_as.sgml b/doc/src/sgml/ref/create_table_as.sgml
index 60300ff..8e4ada7 100644
--- a/doc/src/sgml/ref/create_table_as.sgml
+++ b/doc/src/sgml/ref/create_table_as.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE <replaceable>table_name</replaceable>
+CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXISTS ] <replaceable>table_name</replaceable>
[ (<replaceable>column_name</replaceable> [, ...] ) ]
[ WITH ( <replaceable class="PARAMETER">storage_parameter</replaceable> [= <replaceable class="PARAMETER">value</replaceable>] [, ... ] ) | WITH OIDS | WITHOUT OIDS ]
[ ON COMMIT { PRESERVE ROWS | DELETE ROWS | DROP } ]
@@ -91,6 +91,17 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE <replaceable
</varlistentry>
<varlistentry>
+ <term><literal>IF NOT EXISTS</></term>
+ <listitem>
+ <para>
+ Do not throw an error if a relation with the same name already exists.
+ A notice is issued in this case. Refer to <xref linkend="sql-createtable">
+ for details.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><replaceable>table_name</replaceable></term>
<listitem>
<para>
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index e381c06..6f27f39 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -27,6 +27,7 @@
#include "access/htup_details.h"
#include "access/sysattr.h"
#include "access/xact.h"
+#include "catalog/namespace.h"
#include "catalog/toasting.h"
#include "commands/createas.h"
#include "commands/matview.h"
@@ -85,6 +86,22 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
QueryDesc *queryDesc;
ScanDirection dir;
+ if (stmt->if_not_exists)
+ {
+ Oid nspid;
+
+ nspid = RangeVarGetCreationNamespace(stmt->into->rel);
+
+ if (get_relname_relid(stmt->into->rel->relname, nspid))
+ {
+ ereport(NOTICE,
+ (errcode(ERRCODE_DUPLICATE_TABLE),
+ errmsg("relation \"%s\" already exists, skipping",
+ stmt->into->rel->relname)));
+ return InvalidOid;
+ }
+ }
+
/*
* Create the tuple receiver object and insert info it will need
*/
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 21b070a..3815905 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3286,6 +3286,7 @@ _copyCreateTableAsStmt(const CreateTableAsStmt *from)
COPY_NODE_FIELD(into);
COPY_SCALAR_FIELD(relkind);
COPY_SCALAR_FIELD(is_select_into);
+ COPY_SCALAR_FIELD(if_not_exists);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 358395f..769fbe5 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1529,6 +1529,7 @@ _equalCreateTableAsStmt(const CreateTableAsStmt *a, const CreateTableAsStmt *b)
COMPARE_NODE_FIELD(into);
COMPARE_SCALAR_FIELD(relkind);
COMPARE_SCALAR_FIELD(is_select_into);
+ COMPARE_SCALAR_FIELD(if_not_exists);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0de9584..df1609d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3401,11 +3401,25 @@ CreateAsStmt:
ctas->into = $4;
ctas->relkind = OBJECT_TABLE;
ctas->is_select_into = false;
+ ctas->if_not_exists = false;
/* cram additional flags into the IntoClause */
$4->rel->relpersistence = $2;
$4->skipData = !($7);
$$ = (Node *) ctas;
}
+ | CREATE OptTemp TABLE IF_P NOT EXISTS create_as_target AS SelectStmt opt_with_data
+ {
+ CreateTableAsStmt *ctas = makeNode(CreateTableAsStmt);
+ ctas->query = $9;
+ ctas->into = $7;
+ ctas->relkind = OBJECT_TABLE;
+ ctas->is_select_into = false;
+ ctas->if_not_exists = true;
+ /* cram additional flags into the IntoClause */
+ $7->rel->relpersistence = $2;
+ $7->skipData = !($10);
+ $$ = (Node *) ctas;
+ }
;
create_as_target:
@@ -3444,11 +3458,25 @@ CreateMatViewStmt:
ctas->into = $5;
ctas->relkind = OBJECT_MATVIEW;
ctas->is_select_into = false;
+ ctas->if_not_exists = false;
/* cram additional flags into the IntoClause */
$5->rel->relpersistence = $2;
$5->skipData = !($8);
$$ = (Node *) ctas;
}
+ | CREATE OptNoLog MATERIALIZED VIEW IF_P NOT EXISTS create_mv_target AS SelectStmt opt_with_data
+ {
+ CreateTableAsStmt *ctas = makeNode(CreateTableAsStmt);
+ ctas->query = $10;
+ ctas->into = $8;
+ ctas->relkind = OBJECT_MATVIEW;
+ ctas->is_select_into = false;
+ ctas->if_not_exists = true;
+ /* cram additional flags into the IntoClause */
+ $8->rel->relpersistence = $2;
+ $8->skipData = !($11);
+ $$ = (Node *) ctas;
+ }
;
create_mv_target:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index cef9544..d9b8d61 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2651,6 +2651,7 @@ typedef struct CreateTableAsStmt
IntoClause *into; /* destination table */
ObjectType relkind; /* OBJECT_TABLE or OBJECT_MATVIEW */
bool is_select_into; /* it was written as SELECT INTO */
+ bool if_not_exists; /* just do nothing if it already exists? */
} CreateTableAsStmt;
/* ----------------------
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 167d02d..35451d5 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -218,3 +218,9 @@ CREATE TEMP TABLE pg_temp.doubly_temp (a int primary key); -- also OK
CREATE TEMP TABLE public.temp_to_perm (a int primary key); -- not OK
ERROR: cannot create temporary relation in non-temporary schema
DROP TABLE unlogged1, public.unlogged2;
+CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r';
+CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r';
+ERROR: relation "as_select1" already exists
+CREATE TABLE IF NOT EXISTS as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r';
+NOTICE: relation "as_select1" already exists, skipping
+DROP TABLE as_select1;
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index b04510c..eb13ea7 100644
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -508,6 +508,10 @@ SET ROLE user_dw;
CREATE TABLE foo_data AS SELECT i, md5(random()::text)
FROM generate_series(1, 10) i;
CREATE MATERIALIZED VIEW mv_foo AS SELECT * FROM foo_data;
+CREATE MATERIALIZED VIEW mv_foo AS SELECT * FROM foo_data;
+ERROR: relation "mv_foo" already exists
+CREATE MATERIALIZED VIEW IF NOT EXISTS mv_foo AS SELECT * FROM foo_data;
+NOTICE: relation "mv_foo" already exists, skipping
CREATE UNIQUE INDEX ON mv_foo (i);
RESET ROLE;
REFRESH MATERIALIZED VIEW mv_foo;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 8eb246b..08029a9 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -254,3 +254,8 @@ CREATE TEMP TABLE explicitly_temp (a int primary key); -- also OK
CREATE TEMP TABLE pg_temp.doubly_temp (a int primary key); -- also OK
CREATE TEMP TABLE public.temp_to_perm (a int primary key); -- not OK
DROP TABLE unlogged1, public.unlogged2;
+
+CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r';
+CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r';
+CREATE TABLE IF NOT EXISTS as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r';
+DROP TABLE as_select1;
diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index fee1ddc..70e4492 100644
--- a/src/test/regress/sql/matview.sql
+++ b/src/test/regress/sql/matview.sql
@@ -201,6 +201,8 @@ SET ROLE user_dw;
CREATE TABLE foo_data AS SELECT i, md5(random()::text)
FROM generate_series(1, 10) i;
CREATE MATERIALIZED VIEW mv_foo AS SELECT * FROM foo_data;
+CREATE MATERIALIZED VIEW mv_foo AS SELECT * FROM foo_data;
+CREATE MATERIALIZED VIEW IF NOT EXISTS mv_foo AS SELECT * FROM foo_data;
CREATE UNIQUE INDEX ON mv_foo (i);
RESET ROLE;
REFRESH MATERIALIZED VIEW mv_foo;
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
