Re: warning to publication created and wal_level is not set to logical

2019-07-12 Thread Lucas Viecelli
> Agreed, fixed.  Also run through pgindent
>

Thank you for the adjustments.


> I agree that it's not really worth having tests for this, and I take
> your point about the dependency on wal_level that we don't currently
> have.  The problem is that the core tests include publications
> already, and it doesn't seem like a great idea to move the whole lot
> to a TAP test.  Creating alternative expected files seems like a bad
> idea too (annoying to maintain, wouldn't compose well with the next
> thing like this).  So... how about we just suppress WARNINGs for
> CREATE PUBLICATION commands that are expected to succeed?  Like in the
> attached.  This version passes installcheck with any wal_level.
>
All right, for me. If wal_level can not interfere with the testes result,
it seems to a better approach

*Lucas Viecelli*


<http://www.leosoft.com.br/coopcred>


Re: warning to publication created and wal_level is not set to logical

2019-07-08 Thread Lucas Viecelli
Follow the correct file, I added the wrong patch in the previous email


> Attached is the rebased
>


thanks a lot

*Lucas Viecelli*
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 1ac1a71bd9..902180cedc 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -232,6 +232,14 @@ CreatePublication(CreatePublicationStmt *stmt)
 
 	InvokeObjectPostCreateHook(PublicationRelationId, puboid, 0);
 
+	if (wal_level != WAL_LEVEL_LOGICAL)
+	{
+		ereport(WARNING,
+			(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+			errmsg("insufficient wal_level to publish logical changes"),
+			errhint("Set wal_level to logical before creating subscriptions")));
+	}
+
 	return myself;
 }
 
@@ -763,4 +771,4 @@ AlterPublicationOwner_oid(Oid subid, Oid newOwnerId)
 	heap_freetuple(tup);
 
 	table_close(rel, RowExclusiveLock);
-}
+}
\ No newline at end of file
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index cdbde29502..18daf0d7b6 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -43,6 +43,8 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
 	FROM SQL WITH FUNCTION prsd_lextype(internal),
 	TO SQL WITH FUNCTION int4recv(internal));
 CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 CREATE SUBSCRIPTION regress_addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
 WARNING:  tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
 CREATE STATISTICS addr_nsp.gentable_stat ON a, b FROM addr_nsp.gentable;
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 0e5e8f2b92..70d7119f21 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -5,7 +5,15 @@ CREATE ROLE regress_publication_user LOGIN SUPERUSER;
 CREATE ROLE regress_publication_user2;
 CREATE ROLE regress_publication_user_dummy LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_publication_user';
+SHOW wal_level;
+ wal_level 
+---
+ replica
+(1 row)
+
 CREATE PUBLICATION testpub_default;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 COMMENT ON PUBLICATION testpub_default IS 'test publication';
 SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
  obj_description  
@@ -14,6 +22,8 @@ SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
 (1 row)
 
 CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 ALTER PUBLICATION testpub_default SET (publish = update);
 -- error cases
 CREATE PUBLICATION testpub_xxx WITH (foo);
@@ -44,6 +54,8 @@ CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
 CREATE VIEW testpub_view AS SELECT 1;
 CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
 CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 ALTER PUBLICATION testpub_foralltables SET (publish = 'insert, update');
 CREATE TABLE testpub_tbl2 (id serial primary key, data text);
 -- fail - can't add to for all tables publication
@@ -87,7 +99,11 @@ DROP PUBLICATION testpub_foralltables;
 CREATE TABLE testpub_tbl3 (a int);
 CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3);
 CREATE PUBLICATION testpub3 FOR TABLE testpub_tbl3;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 \dRp+ testpub3
   Publication testpub3
   Owner   | All tables | Inserts | Updates | Deletes | Truncates 
@@ -112,6 +128,8 @@ CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
 ERROR:  "testpub_view" is not a table
 DETAIL:  Only tables can be added to publications.
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 -- fail - already added
 ALTER PUBLICATION testpub_fortbl ADD TABLE testpub_tbl1;
 ERROR:  relation "testpub_tbl1" is already member of publication "testpub_fortbl"

Re: warning to publication created and wal_level is not set to logical

2019-07-08 Thread Lucas Viecelli
Hi Thomas.

Attached is the rebased


> The July Commitfest has started.  This patch is in "Needs review"
> status, but it doesn't apply.  If I read the above discussion
> correctly, it seems there is agreement that a warning here is a good
> idea to commit this patch.  Could you please post a rebased patch?
>
>
I followed your suggestion and changed the message and added HINT. I hope
everything is agreed now.


> I wonder if it would be more typical project style to put the clue on
> what to do into an "errhint" message, something like this:
>
> WARNING: insufficient wal_level to publish logical changes
> HINT:  Set wal_level to logical before creating subscriptions.
>

-- 

*Lucas Viecelli*


<http://www.leosoft.com.br/coopcred>
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 4d48be0b92..5d18146c25 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -232,6 +232,14 @@ CreatePublication(CreatePublicationStmt *stmt)
 
 	InvokeObjectPostCreateHook(PublicationRelationId, puboid, 0);
 
+	if (wal_level != WAL_LEVEL_LOGICAL)
+	{
+		ereport(WARNING,
+			(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+			errmsg("insufficient wal_level to publish logical changes"),
+			errhint("Set wal_level to logical before creating subscriptions")));
+	}
+
 	return myself;
 }
 
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 02070fd8af..e4d1a63e3f 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -43,6 +43,8 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
 	FROM SQL WITH FUNCTION prsd_lextype(internal),
 	TO SQL WITH FUNCTION int4recv(internal));
 CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
 WARNING:  tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
 CREATE STATISTICS addr_nsp.gentable_stat ON a, b FROM addr_nsp.gentable;
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index afbbdd543d..a2d85a3f7f 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -5,7 +5,15 @@ CREATE ROLE regress_publication_user LOGIN SUPERUSER;
 CREATE ROLE regress_publication_user2;
 CREATE ROLE regress_publication_user_dummy LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_publication_user';
+SHOW wal_level;
+ wal_level 
+---
+ replica
+(1 row)
+
 CREATE PUBLICATION testpub_default;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 COMMENT ON PUBLICATION testpub_default IS 'test publication';
 SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
  obj_description  
@@ -14,6 +22,8 @@ SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
 (1 row)
 
 CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 ALTER PUBLICATION testpub_default SET (publish = update);
 -- error cases
 CREATE PUBLICATION testpub_xxx WITH (foo);
@@ -44,6 +54,8 @@ CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
 CREATE VIEW testpub_view AS SELECT 1;
 CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
 CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 ALTER PUBLICATION testpub_foralltables SET (publish = 'insert, update');
 CREATE TABLE testpub_tbl2 (id serial primary key, data text);
 -- fail - can't add to for all tables publication
@@ -87,7 +99,11 @@ DROP PUBLICATION testpub_foralltables;
 CREATE TABLE testpub_tbl3 (a int);
 CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3);
 CREATE PUBLICATION testpub3 FOR TABLE testpub_tbl3;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
+WARNING:  insufficient wal_level to publish logical changes
+HINT:  Set wal_level to logical before creating subscriptions
 \dRp+ testpub3
   Publication testpub3
   Owner   | All tables | Inserts | Updates | Deletes | Truncates 
@@ -112,6 +128,8 @@ CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
 ERROR:  "test

Re: warning to publication created and wal_level is not set to logical

2019-03-26 Thread Lucas Viecelli
>> One idea that might be useful is to have walsenders refuse to transmit
> >> any logical-replication data if they see wal_level is too low.  That
> >> would get users' attention pretty quickly.
>
> > They do:
>

I checked this before creating the patch


>
> Oh, OK, then this seems like it's basically covered already.  I think
> the original suggestion to add a WARNING during CREATE PUBLICATION
> isn't unreasonable.  But we don't need to do more than that (and it
> shouldn't be higher than WARNING).
>

Okay, I think it will improve understanding of new users.

Since everything is fine, thank you all for the comments
-- 

Atenciosamente.

*Lucas Viecelli*

<http://www.leosoft.com.br/coopcred>


Re: warning to publication created and wal_level is not set to logical

2019-03-25 Thread Lucas Viecelli
>
> > Is a WARNING sufficient?  Maybe I'm misunderstanding something
>
> important, but I think the attempt should fail with a HINT to set the
> > wal_level ahead of time.
>

I thought about this possibility, but I was afraid to change the current
behavior a lot, but it's worth discussing.


>
>
I agree that it'd be nice to be noisier about the problem, but I'm
> not sure we can do more than bleat in the postmaster log from time
> to time if a publication is active and wal_level is too low.
> (And we'd better be careful about the log-spam aspect of that...)
>

I agree on being noisier, but I think the main thing is to let the user
aware of the situation and in that the
patch resolves, stating that he needs to adjust wal_level. Initially
WARNING will appear only at the time
the publication is created, precisely not to put spam in the log.

Is it better to warn from time to time that wal_level needs to change
because it has some publication that will not work?
-- 

*Lucas Viecelli*


<http://www.leosoft.com.br/coopcred>


warning to publication created and wal_level is not set to logical

2019-03-21 Thread Lucas Viecelli
Hi everyone,

A very common question among new users is how wal_level works and it
levels. I heard about some situations like that, a user create a new
publication in its master database and he/she simply does not change
wal_level to logical, sometimes, this person lost maintenance window, or a
chance to restart postgres service, usually a production database, and it
will discover that wal_level is not right just in subscription creation.
Attempting to iterate between new (and even experienced) users with logical
replication, I am sending a patch that when an PUBLICATION is created and
the wal_level is different from logical prints a WARNING in console/log:

-> WARNING: `PUBLICATION` created but wal_level `is` not set to logical,
you need to change it before creating any SUBSCRIPTION

Initiatives like this can make a good user experience with PostgreSQL and
its own logical replication.

Thanks

--

*Lucas Viecelli*

<http://www.leosoft.com.br/coopcred>
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 4d48be0b92..991065d0a2 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -232,6 +232,12 @@ CreatePublication(CreatePublicationStmt *stmt)
 
 	InvokeObjectPostCreateHook(PublicationRelationId, puboid, 0);
 
+	if (wal_level != WAL_LEVEL_LOGICAL)
+	{
+		elog(WARNING, "PUBLICATION created but wal_level is not set to logical, you need to "
+  "change it before creating any SUBSCRIPTION");
+	}
+
 	return myself;
 }
 
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index 02070fd8af..dbc3ffa63d 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -43,6 +43,7 @@ CREATE TRANSFORM FOR int LANGUAGE SQL (
 	FROM SQL WITH FUNCTION prsd_lextype(internal),
 	TO SQL WITH FUNCTION int4recv(internal));
 CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
+WARNING:  PUBLICATION created but wal_level is not set to logical, you need to change it before creating any SUBSCRIPTION
 CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
 WARNING:  tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
 CREATE STATISTICS addr_nsp.gentable_stat ON a, b FROM addr_nsp.gentable;
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index afbbdd543d..2b05babf7a 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -5,7 +5,14 @@ CREATE ROLE regress_publication_user LOGIN SUPERUSER;
 CREATE ROLE regress_publication_user2;
 CREATE ROLE regress_publication_user_dummy LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_publication_user';
+SHOW wal_level;
+ wal_level 
+---
+ replica
+(1 row)
+
 CREATE PUBLICATION testpub_default;
+WARNING:  PUBLICATION created but wal_level is not set to logical, you need to change it before creating any SUBSCRIPTION
 COMMENT ON PUBLICATION testpub_default IS 'test publication';
 SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
  obj_description  
@@ -14,6 +21,7 @@ SELECT obj_description(p.oid, 'pg_publication') FROM pg_publication p;
 (1 row)
 
 CREATE PUBLICATION testpib_ins_trunct WITH (publish = insert);
+WARNING:  PUBLICATION created but wal_level is not set to logical, you need to change it before creating any SUBSCRIPTION
 ALTER PUBLICATION testpub_default SET (publish = update);
 -- error cases
 CREATE PUBLICATION testpub_xxx WITH (foo);
@@ -44,6 +52,7 @@ CREATE TABLE pub_test.testpub_nopk (foo int, bar int);
 CREATE VIEW testpub_view AS SELECT 1;
 CREATE TABLE testpub_parted (a int) PARTITION BY LIST (a);
 CREATE PUBLICATION testpub_foralltables FOR ALL TABLES WITH (publish = 'insert');
+WARNING:  PUBLICATION created but wal_level is not set to logical, you need to change it before creating any SUBSCRIPTION
 ALTER PUBLICATION testpub_foralltables SET (publish = 'insert, update');
 CREATE TABLE testpub_tbl2 (id serial primary key, data text);
 -- fail - can't add to for all tables publication
@@ -87,7 +96,9 @@ DROP PUBLICATION testpub_foralltables;
 CREATE TABLE testpub_tbl3 (a int);
 CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3);
 CREATE PUBLICATION testpub3 FOR TABLE testpub_tbl3;
+WARNING:  PUBLICATION created but wal_level is not set to logical, you need to change it before creating any SUBSCRIPTION
 CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
+WARNING:  PUBLICATION created but wal_level is not set to logical, you need to change it before creating any SUBSCRIPTION
 \dRp+ testpub3
   Publication testpub3
   Owner   | All tables | Inserts | Updates | D