Not getting error if ALTER SUBSCRIPTION syntax is wrong.

Started by tusharover 8 years ago6 messages
#1tushar
tushar.ahuja@enterprisedb.com

Hi,

While testing 'logical replication' against v10 , i encountered one
issue where data stop migrating after ALTER PUBLICATION.

X Server
\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table/Insert 1 row -> create table test(n int); insert into t
values (1);
\\create publication for all -> create publication pub for ALL TABLES ;

Y server

\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table -> create table test(n int);

\\create Subscription

CREATE SUBSCRIPTION sub CONNECTION 'host=localhost dbname=postgres
port=5432 ' PUBLICATION pub;

postgres=# select * from test;
n
---
1
(1 row)

\\Alter subscription
postgres=# alter subscription sub connection 'host=localhost
dbname=postgres PUBLICATION pub';
ALTER SUBSCRIPTION

X server
postgres=# insert into test values (1);
INSERT 0 1
postgres=# select * from test;
n
---
1
1
(2 rows)

Y server
postgres=# select * from test;
n
---
1
(1 row)

I think probably syntax of alter subscription is not correct but
surprisingly it is not throwing an error.

--
regards,tushar
EnterpriseDB https://www.enterprisedb.com/
The Enterprise PostgreSQL Company

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

#2Petr Jelinek
petr.jelinek@2ndquadrant.com
In reply to: tushar (#1)
Re: Not getting error if ALTER SUBSCRIPTION syntax is wrong.

On 05/05/17 14:40, tushar wrote:

Hi,

While testing 'logical replication' against v10 , i encountered one
issue where data stop migrating after ALTER PUBLICATION.

X Server
\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table/Insert 1 row -> create table test(n int); insert into t
values (1);
\\create publication for all -> create publication pub for ALL TABLES ;

Y server

\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table -> create table test(n int);

\\create Subscription

CREATE SUBSCRIPTION sub CONNECTION 'host=localhost dbname=postgres
port=5432 ' PUBLICATION pub;

postgres=# select * from test;
n
---
1
(1 row)

\\Alter subscription
postgres=# alter subscription sub connection 'host=localhost
dbname=postgres PUBLICATION pub';
ALTER SUBSCRIPTION

X server
postgres=# insert into test values (1);
INSERT 0 1
postgres=# select * from test;
n
---
1
1
(2 rows)

Y server
postgres=# select * from test;
n
---
1
(1 row)

I think probably syntax of alter subscription is not correct but
surprisingly it is not throwing an error.

Syntax of ALTER command is correct, syntax of the connection string is
not, you are probably getting errors in log from the replication worker.

We could check validity of the connection string though to complain
immediately like we do in CREATE.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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

#3Robert Haas
robertmhaas@gmail.com
In reply to: Petr Jelinek (#2)
Re: Not getting error if ALTER SUBSCRIPTION syntax is wrong.

On Fri, May 5, 2017 at 1:51 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

Syntax of ALTER command is correct, syntax of the connection string is
not, you are probably getting errors in log from the replication worker.

We could check validity of the connection string though to complain
immediately like we do in CREATE.

Well if CREATE checks the validity, surely it's a bug if ALTER doesn't
do the same.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

#4Petr Jelinek
petr.jelinek@2ndquadrant.com
In reply to: Petr Jelinek (#2)
1 attachment(s)
Re: Not getting error if ALTER SUBSCRIPTION syntax is wrong.

On 05/05/17 19:51, Petr Jelinek wrote:

On 05/05/17 14:40, tushar wrote:

Hi,

While testing 'logical replication' against v10 , i encountered one
issue where data stop migrating after ALTER PUBLICATION.

X Server
\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table/Insert 1 row -> create table test(n int); insert into t
values (1);
\\create publication for all -> create publication pub for ALL TABLES ;

Y server

\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table -> create table test(n int);

\\create Subscription

CREATE SUBSCRIPTION sub CONNECTION 'host=localhost dbname=postgres
port=5432 ' PUBLICATION pub;

[...]

I think probably syntax of alter subscription is not correct but
surprisingly it is not throwing an error.

Syntax of ALTER command is correct, syntax of the connection string is
not, you are probably getting errors in log from the replication worker.

We could check validity of the connection string though to complain
immediately like we do in CREATE.

The attached does exactly that.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

Check-connection-info-in-ALTER-SUBSCRIPTION.patchbinary/octet-stream; name=Check-connection-info-in-ALTER-SUBSCRIPTION.patchDownload
From 2505a0e516457ec71414674ac2a33553fe7a691a Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Sat, 6 May 2017 14:28:28 +0200
Subject: [PATCH] Check connection info in ALTER SUBSCRIPTION

---
 src/backend/commands/subscriptioncmds.c    | 5 +++++
 src/test/regress/expected/subscription.out | 4 ++++
 src/test/regress/sql/subscription.sql      | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index afb116a..a9b3f77 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -673,6 +673,11 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
 			}
 
 		case ALTER_SUBSCRIPTION_CONNECTION:
+			/* Load the library providing us libpq calls. */
+			load_file("libpqwalreceiver", false);
+			/* Check the connection info string. */
+			walrcv_check_conninfo(stmt->conninfo);
+
 			values[Anum_pg_subscription_subconninfo - 1] =
 				CStringGetTextDatum(stmt->conninfo);
 			replaces[Anum_pg_subscription_subconninfo - 1] = true;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index fcf5646..4821113 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -45,6 +45,10 @@ SET SESSION AUTHORIZATION 'regress_subscription_user2';
 CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
 ERROR:  must be superuser to create subscriptions
 SET SESSION AUTHORIZATION 'regress_subscription_user';
+-- fail - invalid connection string
+ALTER SUBSCRIPTION testsub CONNECTION 'foobar';
+ERROR:  invalid connection string syntax: missing "=" after "foobar" in connection info string
+
 \dRs+
                                          List of subscriptions
   Name   |           Owner           | Enabled | Publication | Synchronous commit |      Conninfo       
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index c27fee5..e5affea 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -38,6 +38,9 @@ SET SESSION AUTHORIZATION 'regress_subscription_user2';
 CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
 SET SESSION AUTHORIZATION 'regress_subscription_user';
 
+-- fail - invalid connection string
+ALTER SUBSCRIPTION testsub CONNECTION 'foobar';
+
 \dRs+
 
 ALTER SUBSCRIPTION testsub SET PUBLICATION testpub2, testpub3 SKIP REFRESH;
-- 
2.7.4

#5Noah Misch
noah@leadboat.com
In reply to: Petr Jelinek (#4)
Re: Not getting error if ALTER SUBSCRIPTION syntax is wrong.

On Sat, May 06, 2017 at 02:44:27PM +0200, Petr Jelinek wrote:

On 05/05/17 19:51, Petr Jelinek wrote:

On 05/05/17 14:40, tushar wrote:

Hi,

While testing 'logical replication' against v10 , i encountered one
issue where data stop migrating after ALTER PUBLICATION.

X Server
\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table/Insert 1 row -> create table test(n int); insert into t
values (1);
\\create publication for all -> create publication pub for ALL TABLES ;

Y server

\\ Make sure wal_level is set to logical in postgresql.conf file
\\create table -> create table test(n int);

\\create Subscription

CREATE SUBSCRIPTION sub CONNECTION 'host=localhost dbname=postgres
port=5432 ' PUBLICATION pub;

[...]

I think probably syntax of alter subscription is not correct but
surprisingly it is not throwing an error.

Syntax of ALTER command is correct, syntax of the connection string is
not, you are probably getting errors in log from the replication worker.

We could check validity of the connection string though to complain
immediately like we do in CREATE.

The attached does exactly that.

[Action required within three days. This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.

[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com

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

#6Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Petr Jelinek (#4)
Re: Not getting error if ALTER SUBSCRIPTION syntax is wrong.

On 5/6/17 08:44, Petr Jelinek wrote:

We could check validity of the connection string though to complain
immediately like we do in CREATE.

The attached does exactly that.

committed

--
Peter Eisentraut http://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