Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Arthur Zakirov]

2017-03-14 15:55 GMT+03:00 Ashutosh Bapat :
>
> I noticed that the earlier error message was using "server" instead of
> "foreign server", while the new message uses the later one. Usually,
> when converting an error to notice, we don't expect such changes. But
> many other error messages are using "foreign server" instead of
> "server", so probably this one needed a change anyway. But then, the
> command to create a foreign server is not "CREATE FOREIGN SERVER",
> it's "CREATE SERVER", so users are already getting confused?

Actually, there are other messages with "server". For example, in the
AlterForeignServerOwner() or in the postgres_fdw code.
Maybe it is better to not change "server" to "foreign server" in
"create_foreign_server_if_not_exists.patch"? I think it will be better
to fix all such messages with a separate patch, If we will decide that
it is necessary to change "server" messages.

>
> I don't see similar change in the error message for the user mapping.
> Do we need to change "server" to "foreign server" in case of user
> mapping?  The doc changes didn't compile with error
> "osx:ref/create_user_mapping.sgml:52:15:E: document type does not
> allow element "VARLISTENTRY" here; assuming missing "VARIABLELIST"

Indeed! Missed that.


-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Ashutosh Bapat]

>
> Thanks for catching that.
> It was caused by a conflict on applying of the patch.
> Updated versions of both patches are attached.
>

We do not need extra line
   
+
+  
other usages of this do not have an extra line. Removed the extra line
in the attached patch.

I noticed that the earlier error message was using "server" instead of
"foreign server", while the new message uses the later one. Usually,
when converting an error to notice, we don't expect such changes. But
many other error messages are using "foreign server" instead of
"server", so probably this one needed a change anyway. But then, the
command to create a foreign server is not "CREATE FOREIGN SERVER",
it's "CREATE SERVER", so users are already getting confused?

I don't see similar change in the error message for the user mapping.
Do we need to change "server" to "foreign server" in case of user
mapping?  The doc changes didn't compile with error
"osx:ref/create_user_mapping.sgml:52:15:E: document type does not
allow element "VARLISTENTRY" here; assuming missing "VARIABLELIST"
start-tag". The user mappings do not have name so the doc change was
slightly incorrect when it said "Do not throw an error if a user
mapping with the same name already exists.". I have corrected both
these things in the attached patch.

Other changes look good.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


create_user_mapping_if_not_exists_3.0.patch
Description: Binary data


create_foreign_server_if_not_exists_3.0.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Artur Zakirov]

On 13.03.2017 13:01, Anastasia Lubennikova wrote:


Thanks for catching that.
It was caused by a conflict on applying of the patch.
Updated versions of both patches are attached.



I think the code is good and the patches are small. Documentation is 
updated by the patches.


All regression tests are passed.

Marked the patch as "Ready for Commiter".

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Anastasia Lubennikova]

13.03.2017 11:53, Artur Zakirov:

On 15.02.2017 20:54, Anastasia Lubennikova wrote:


Done.



I have gotten the error that AlterUserMappingStmt doesn't have 
if_not_exists (in Russian):



gram.y: В функции «base_yyparse»:
gram.y:4918:7: ошибка: «AlterUserMappingStmt {aka struct 
AlterUserMappingStmt}» не содержит элемента с именем «if_not_exists»

  n->if_not_exists = false;
   ^~


After applying the CREATE USER patch in gram.y I have:



AlterUserMappingStmt: ALTER USER MAPPING FOR auth_ident SERVER name 
alter_generic_options

{
AlterUserMappingStmt *n = 
makeNode(AlterUserMappingStmt);

n->user = $5;
n->servername = $7;
n->options = $8;
n->if_not_exists = false;
$$ = (Node *) n;
}
| CREATE USER MAPPING IF_P NOT EXISTS FOR auth_ident 
SERVER name create_generic_options

{
CreateUserMappingStmt *n = 
makeNode(CreateUserMappingStmt);

n->user = $8;
n->servername = $10;
n->options = $11;
n->if_not_exists = true;
$$ = (Node *) n;
}
;


Here ALTER USER MAPPING and CREATE USER MAPPING commands were mixed.



Thanks for catching that.
It was caused by a conflict on applying of the patch.
Updated versions of both patches are attached.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

diff --git a/doc/src/sgml/ref/create_server.sgml b/doc/src/sgml/ref/create_server.sgml
index 734c6c9..6777679 100644
--- a/doc/src/sgml/ref/create_server.sgml
+++ b/doc/src/sgml/ref/create_server.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-CREATE SERVER server_name [ TYPE 'server_type' ] [ VERSION 'server_version' ]
+CREATE SERVER [IF NOT EXISTS] server_name [ TYPE 'server_type' ] [ VERSION 'server_version' ]
 FOREIGN DATA WRAPPER fdw_name
 [ OPTIONS ( option 'value' [, ... ] ) ]
 
@@ -56,6 +56,19 @@ CREATE SERVER server_name [ TYPE '<
   Parameters
 
   
+
+  
+IF NOT EXISTS
+
+ 
+  Do not throw an error if a server with the same name already exists.
+  A notice is issued in this case.  Note that there is no guarantee that
+  the existing server is anything like the one that would have been
+  created.
+ 
+
+   
+

 server_name
 
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index d5d40e6..41b2c01 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -879,12 +879,25 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
 
 	/*
 	 * Check that there is no other foreign server by this name.
+	 * Do nothing if IF NOT EXISTS was enforced.
 	 */
 	if (GetForeignServerByName(stmt->servername, true) != NULL)
-		ereport(ERROR,
-(errcode(ERRCODE_DUPLICATE_OBJECT),
- errmsg("server \"%s\" already exists",
-		stmt->servername)));
+	{
+		if (stmt->if_not_exists)
+		{
+			ereport(NOTICE,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("foreign server \"%s\" already exists, skipping",
+			stmt->servername)));
+			heap_close(rel, RowExclusiveLock);
+			return InvalidObjectAddress;
+		}
+		else
+			ereport(ERROR,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("foreign server \"%s\" already exists",
+			stmt->servername)));
+	}
 
 	/*
 	 * Check that the FDW exists and that we have USAGE on it. Also get the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e7acc2d..da67b51 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4621,6 +4621,19 @@ CreateForeignServerStmt: CREATE SERVER name opt_type opt_foreign_server_version
 	n->version = $5;
 	n->fdwname = $9;
 	n->options = $10;
+	n->if_not_exists = false;
+	$$ = (Node *) n;
+}
+| CREATE SERVER IF_P NOT EXISTS name opt_type opt_foreign_server_version
+		 FOREIGN DATA_P WRAPPER name create_generic_options
+{
+	CreateForeignServerStmt *n = makeNode(CreateForeignServerStmt);
+	n->servername = $6;
+	n->servertype = $7;
+	n->version = $8;
+	n->fdwname = $12;
+	n->options = $13;
+	n->if_not_exists = true;
 	$$ = (Node *) n;
 }
 		;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index a44d217..804436b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2151,6 +2151,7 @@ typedef struct CreateForeignServerStmt
 	char	   *servertype;		/* optional server type */
 	char	   *version;		/* optional server version */
 	char	   *fdwname;		/* FDW name */
+	bool		if_not_exists;	/* just do nothing if it already exists? */
 	List	   *options;		/* generic options to server */
 } CreateForeignServerStmt;
 
diff --git a/src/test/regress/expected/foreign_data.out 

Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Artur Zakirov]

On 15.02.2017 20:54, Anastasia Lubennikova wrote:


Done.



I have gotten the error that AlterUserMappingStmt doesn't have 
if_not_exists (in Russian):



gram.y: В функции «base_yyparse»:
gram.y:4918:7: ошибка: «AlterUserMappingStmt {aka struct AlterUserMappingStmt}» 
не содержит элемента с именем «if_not_exists»
  n->if_not_exists = false;
   ^~


After applying the CREATE USER patch in gram.y I have:



AlterUserMappingStmt: ALTER USER MAPPING FOR auth_ident SERVER name 
alter_generic_options
{
AlterUserMappingStmt *n = makeNode(AlterUserMappingStmt);
n->user = $5;
n->servername = $7;
n->options = $8;
n->if_not_exists = false;
$$ = (Node *) n;
}
| CREATE USER MAPPING IF_P NOT EXISTS FOR auth_ident SERVER 
name create_generic_options
{
CreateUserMappingStmt *n = makeNode(CreateUserMappingStmt);
n->user = $8;
n->servername = $10;
n->options = $11;
n->if_not_exists = true;
$$ = (Node *) n;
}
;


Here ALTER USER MAPPING and CREATE USER MAPPING commands were mixed.

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Anastasia Lubennikova]

13.02.2017 19:34, Andrew Dunstan:


On 01/13/2017 08:36 AM, Anastasia Lubennikova wrote:

I implemented IF NOT EXISTS option for CREATE SERVER and CREATE USER
MAPPING statements
for one of our customers.
I think other users can also find it useful for scripting and
automated tasks.
The patches themselves are small and simple. Documentation is updated
as well.




This looks good and useful. Please add some regression tests.


Done.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

diff --git a/doc/src/sgml/ref/create_server.sgml b/doc/src/sgml/ref/create_server.sgml
index 734c6c9..6777679 100644
--- a/doc/src/sgml/ref/create_server.sgml
+++ b/doc/src/sgml/ref/create_server.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-CREATE SERVER server_name [ TYPE 'server_type' ] [ VERSION 'server_version' ]
+CREATE SERVER [IF NOT EXISTS] server_name [ TYPE 'server_type' ] [ VERSION 'server_version' ]
 FOREIGN DATA WRAPPER fdw_name
 [ OPTIONS ( option 'value' [, ... ] ) ]
 
@@ -56,6 +56,19 @@ CREATE SERVER server_name [ TYPE '<
   Parameters
 
   
+
+  
+IF NOT EXISTS
+
+ 
+  Do not throw an error if a server with the same name already exists.
+  A notice is issued in this case.  Note that there is no guarantee that
+  the existing server is anything like the one that would have been
+  created.
+ 
+
+   
+

 server_name
 
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index 6ff8b69..b4ae5de 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -883,12 +883,25 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
 
 	/*
 	 * Check that there is no other foreign server by this name.
+	 * Do nothing if IF NOT EXISTS was enforced.
 	 */
 	if (GetForeignServerByName(stmt->servername, true) != NULL)
-		ereport(ERROR,
-(errcode(ERRCODE_DUPLICATE_OBJECT),
- errmsg("server \"%s\" already exists",
-		stmt->servername)));
+	{
+		if (stmt->if_not_exists)
+		{
+			ereport(NOTICE,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("foreign server \"%s\" already exists, skipping",
+			stmt->servername)));
+			heap_close(rel, RowExclusiveLock);
+			return InvalidObjectAddress;
+		}
+		else
+			ereport(ERROR,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("foreign server \"%s\" already exists",
+			stmt->servername)));
+	}
 
 	/*
 	 * Check that the FDW exists and that we have USAGE on it. Also get the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a4edea0..d007468 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4655,6 +4655,19 @@ CreateForeignServerStmt: CREATE SERVER name opt_type opt_foreign_server_version
 	n->version = $5;
 	n->fdwname = $9;
 	n->options = $10;
+	n->if_not_exists = false;
+	$$ = (Node *) n;
+}
+| CREATE SERVER IF_P NOT EXISTS name opt_type opt_foreign_server_version
+		 FOREIGN DATA_P WRAPPER name create_generic_options
+{
+	CreateForeignServerStmt *n = makeNode(CreateForeignServerStmt);
+	n->servername = $6;
+	n->servertype = $7;
+	n->version = $8;
+	n->fdwname = $12;
+	n->options = $13;
+	n->if_not_exists = true;
 	$$ = (Node *) n;
 }
 		;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 07a8436..704bc6b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2113,6 +2113,7 @@ typedef struct CreateForeignServerStmt
 	char	   *servertype;		/* optional server type */
 	char	   *version;		/* optional server version */
 	char	   *fdwname;		/* FDW name */
+	bool		if_not_exists;	/* just do nothing if it already exists? */
 	List	   *options;		/* generic options to server */
 } CreateForeignServerStmt;
 
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 3a9fb8f..17f9f40 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -283,7 +283,9 @@ ERROR:  foreign-data wrapper "foo" does not exist
 CREATE FOREIGN DATA WRAPPER foo OPTIONS ("test wrapper" 'true');
 CREATE SERVER s1 FOREIGN DATA WRAPPER foo;
 CREATE SERVER s1 FOREIGN DATA WRAPPER foo;  -- ERROR
-ERROR:  server "s1" already exists
+ERROR:  foreign server "s1" already exists
+CREATE SERVER IF NOT EXISTS s1 FOREIGN DATA WRAPPER foo;	-- No ERROR, just NOTICE
+NOTICE:  foreign server "s1" already exists, skipping
 CREATE SERVER s2 FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbname 'b');
 CREATE SERVER s3 TYPE 'oracle' FOREIGN DATA WRAPPER foo;
 CREATE SERVER s4 TYPE 'oracle' FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbname 'b');
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 38e1d41..a1fc10f 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ 

Re: [HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Andrew Dunstan]

On 01/13/2017 08:36 AM, Anastasia Lubennikova wrote:
> I implemented IF NOT EXISTS option for CREATE SERVER and CREATE USER
> MAPPING statements
> for one of our customers.
> I think other users can also find it useful for scripting and
> automated tasks.
> The patches themselves are small and simple. Documentation is updated
> as well.
>



This looks good and useful. Please add some regression tests.

cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements

[Anastasia Lubennikova]

I implemented IF NOT EXISTS option for CREATE SERVER and CREATE USER 
MAPPING statements

for one of our customers.
I think other users can also find it useful for scripting and automated 
tasks.
The patches themselves are small and simple. Documentation is updated as 
well.


--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

commit cf20fa8c9f29310d8f67e5b198a9eb908d903431
Author: Anastasia 
Date:   Fri Jan 13 16:22:01 2017 +0300

Add [IF NOT EXISTS] option to CREATE SERVER statement

diff --git a/doc/src/sgml/ref/create_server.sgml b/doc/src/sgml/ref/create_server.sgml
index 734c6c9..6777679 100644
--- a/doc/src/sgml/ref/create_server.sgml
+++ b/doc/src/sgml/ref/create_server.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-CREATE SERVER server_name [ TYPE 'server_type' ] [ VERSION 'server_version' ]
+CREATE SERVER [IF NOT EXISTS] server_name [ TYPE 'server_type' ] [ VERSION 'server_version' ]
 FOREIGN DATA WRAPPER fdw_name
 [ OPTIONS ( option 'value' [, ... ] ) ]
 
@@ -56,6 +56,19 @@ CREATE SERVER server_name [ TYPE '<
   Parameters
 
   
+
+  
+IF NOT EXISTS
+
+ 
+  Do not throw an error if a server with the same name already exists.
+  A notice is issued in this case.  Note that there is no guarantee that
+  the existing server is anything like the one that would have been
+  created.
+ 
+
+   
+

 server_name
 
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index 06b4bc3..0b0114c 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -883,12 +883,25 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
 
 	/*
 	 * Check that there is no other foreign server by this name.
+	 * Do nothing if IF NOT EXISTS was enforced.
 	 */
 	if (GetForeignServerByName(stmt->servername, true) != NULL)
-		ereport(ERROR,
-(errcode(ERRCODE_DUPLICATE_OBJECT),
- errmsg("server \"%s\" already exists",
-		stmt->servername)));
+	{
+		if (stmt->if_not_exists)
+		{
+			ereport(NOTICE,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("foreign server \"%s\" already exists, skipping",
+			stmt->servername)));
+			heap_close(rel, RowExclusiveLock);
+			return InvalidObjectAddress;
+		}
+		else
+			ereport(ERROR,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("foreign server \"%s\" already exists",
+			stmt->servername)));
+	}
 
 	/*
 	 * Check that the FDW exists and that we have USAGE on it. Also get the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 9eef550..ab4b3b1 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4621,6 +4621,19 @@ CreateForeignServerStmt: CREATE SERVER name opt_type opt_foreign_server_version
 	n->version = $5;
 	n->fdwname = $9;
 	n->options = $10;
+	n->if_not_exists = false;
+	$$ = (Node *) n;
+}
+| CREATE SERVER IF_P NOT EXISTS name opt_type opt_foreign_server_version
+		 FOREIGN DATA_P WRAPPER name create_generic_options
+{
+	CreateForeignServerStmt *n = makeNode(CreateForeignServerStmt);
+	n->servername = $6;
+	n->servertype = $7;
+	n->version = $8;
+	n->fdwname = $12;
+	n->options = $13;
+	n->if_not_exists = true;
 	$$ = (Node *) n;
 }
 		;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 7ceaa22..8a79ec0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2075,6 +2075,7 @@ typedef struct CreateForeignServerStmt
 	char	   *servertype;		/* optional server type */
 	char	   *version;		/* optional server version */
 	char	   *fdwname;		/* FDW name */
+	bool		if_not_exists;	/* just do nothing if it already exists? */
 	List	   *options;		/* generic options to server */
 } CreateForeignServerStmt;
 
commit 4237a23d6674da774b0772ffc953e5f499c04803
Author: Anastasia 
Date:   Fri Jan 13 16:23:53 2017 +0300

Add [IF NOT EXISTS] option to CREATE USER MAPPING statement

diff --git a/doc/src/sgml/ref/create_user_mapping.sgml b/doc/src/sgml/ref/create_user_mapping.sgml
index 44fe302..680906b 100644
--- a/doc/src/sgml/ref/create_user_mapping.sgml
+++ b/doc/src/sgml/ref/create_user_mapping.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  
 
-CREATE USER MAPPING FOR { user_name | USER | CURRENT_USER | PUBLIC }
+CREATE USER MAPPING [IF NOT EXISTS] FOR { user_name | USER | CURRENT_USER | PUBLIC }
 SERVER server_name
 [ OPTIONS ( option 'value' [ , ... ] ) ]
 
@@ -49,6 +49,18 @@ CREATE USER MAPPING FOR { user_name
  
   Parameters
 
+  
+IF NOT EXISTS
+
+ 
+  Do not throw an error if a user mapping with the same name already exists.
+  A notice is issued in this case.  Note that there is no guarantee that
+  the existing user mapping is anything like the one that would have been
+  created.
+ 
+
+