Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-04-07 19:53:56 +0200, Petr Jelinek wrote: > On 07/04/16 12:26, Andres Freund wrote: > >Hi, > > > >On 2016-04-06 20:03:20 +0200, Petr Jelinek wrote: > >>Attached patch adds filtering of both database and origin. Added tests with > >>slightly less hardcoding than what you proposed above. > >

Re: [HACKERS] Proposal: Generic WAL logical messages

On 07/04/16 12:26, Andres Freund wrote: Hi, On 2016-04-06 20:03:20 +0200, Petr Jelinek wrote: Attached patch adds filtering of both database and origin. Added tests with slightly less hardcoding than what you proposed above. Not a fan of creating & dropping another database - that's really

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, On 2016-04-06 20:03:20 +0200, Petr Jelinek wrote: > Attached patch adds filtering of both database and origin. Added tests with > slightly less hardcoding than what you proposed above. Not a fan of creating & dropping another database - that's really rather expensive. And we're in a target

Re: [HACKERS] Proposal: Generic WAL logical messages

On April 7, 2016 2:26:41 AM GMT+02:00, Michael Paquier wrote: >On Thu, Apr 7, 2016 at 12:55 AM, Andres Freund >wrote: >> On 2016-04-06 16:49:17 +0100, Simon Riggs wrote: >>> Perhaps easy to solve, but how do we test it is solved? >> >> Maybe

Re: [HACKERS] Proposal: Generic WAL logical messages

On Thu, Apr 7, 2016 at 12:55 AM, Andres Freund wrote: > On 2016-04-06 16:49:17 +0100, Simon Riggs wrote: >> Perhaps easy to solve, but how do we test it is solved? > > Maybe something like > > -- drain > pg_logical_slot_get_changes(...); > -- generate message in different

Re: [HACKERS] Proposal: Generic WAL logical messages

On 06/04/16 17:55, Andres Freund wrote: On 2016-04-06 16:49:17 +0100, Simon Riggs wrote: Perhaps easy to solve, but how do we test it is solved? Maybe something like -- drain pg_logical_slot_get_changes(...); -- generate message in different database, to ensure it's not processed -- in this

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-04-06 16:49:17 +0100, Simon Riggs wrote: > Perhaps easy to solve, but how do we test it is solved? Maybe something like -- drain pg_logical_slot_get_changes(...); -- generate message in different database, to ensure it's not processed -- in this database \c template1 SELECT

Re: [HACKERS] Proposal: Generic WAL logical messages

On 6 April 2016 at 15:29, Andres Freund wrote: > On 2016-04-06 10:24:52 -0400, Tom Lane wrote: > > Andres Freund writes: > > > On 2016-04-06 10:15:59 -0400, Tom Lane wrote: > > >> Well, that's something worth thinking about. I assume that > > >>

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-04-06 10:24:52 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2016-04-06 10:15:59 -0400, Tom Lane wrote: > >> Well, that's something worth thinking about. I assume that > >> pg_logical_slot_get_changes could be executed in a database different from > >> the one

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-04-06 16:20:29 +0200, Andres Freund wrote: > On 2016-04-06 10:15:59 -0400, Tom Lane wrote: > > > In some ways it seems like the argument to pg_logical_emit_message(...) > > > should > > > be 'bytea'. That'd be much more convenient for application use. But then > > > it's a pain when using

Re: [HACKERS] Proposal: Generic WAL logical messages

Andres Freund writes: > On 2016-04-06 10:15:59 -0400, Tom Lane wrote: >> Well, that's something worth thinking about. I assume that >> pg_logical_slot_get_changes could be executed in a database different from >> the one where a change was originated? > You can execute it,

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-04-06 10:15:59 -0400, Tom Lane wrote: > > In some ways it seems like the argument to pg_logical_emit_message(...) > > should > > be 'bytea'. That'd be much more convenient for application use. But then > > it's a pain when using it via the text-format SQL interface calls, where > > we've

Re: [HACKERS] Proposal: Generic WAL logical messages

Craig Ringer writes: > Interesting issue. Mainly because the "Å¥" char it complains about > (utf-8 0xc5 0xa5) is accepted in the SELECT that generates the record. Uh, no, actually it's the SELECT that's failing. > The regress script in question sets: > SET client_encoding

Re: [HACKERS] Proposal: Generic WAL logical messages

Committed. https://commitfest.postgresql.org/9/468/ Buildfarm error: http://www.postgresql.org/message-id/cab7npqrod2mxqy_5+czjvhw0whrrz6p8jv_rsblcrxrtwlh...@mail.gmail.com Interesting issue. Mainly because the "ť" char it complains about (utf-8 0xc5 0xa5) is accepted in the SELECT that

Re: [HACKERS] Proposal: Generic WAL logical messages

On 4 April 2016 at 05:23, Petr Jelinek wrote: > I rebased this patch on top of current master as the generic wal commit > added some conflicting changes. Also fixed couple of typos in comments and > added non ascii message to test. This looks good to me, so have marked

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, I rebased this patch on top of current master as the generic wal commit added some conflicting changes. Also fixed couple of typos in comments and added non ascii message to test. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support,

Re: [HACKERS] Proposal: Generic WAL logical messages

On 23/03/16 14:17, Alvaro Herrera wrote: Petr Jelinek wrote: +++ b/contrib/test_decoding/sql/messages.sql @@ -0,0 +1,17 @@ +-- predictability +SET synchronous_commit = on; + +SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding'); + +SELECT 'msg1' FROM

Re: [HACKERS] Proposal: Generic WAL logical messages

Petr Jelinek wrote: > +++ b/contrib/test_decoding/sql/messages.sql > @@ -0,0 +1,17 @@ > +-- predictability > +SET synchronous_commit = on; > + > +SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', > 'test_decoding'); > + > +SELECT 'msg1' FROM pg_logical_emit_message(true,

Re: [HACKERS] Proposal: Generic WAL logical messages

On 22/03/16 14:11, Andres Freund wrote: On 2016-03-22 14:03:06 +0100, Petr Jelinek wrote: On 22/03/16 12:47, Andres Freund wrote: On 2016-03-21 18:10:55 +0100, Petr Jelinek wrote: + + + Generic Message Callback + + + The optional message_cb callback is called whenever +

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-03-22 14:03:06 +0100, Petr Jelinek wrote: > On 22/03/16 12:47, Andres Freund wrote: > >On 2016-03-21 18:10:55 +0100, Petr Jelinek wrote: > > > >>+ > >>+ > >>+ Generic Message Callback > >>+ > >>+ > >>+ The optional message_cb callback is called > >>whenever > >>+ a

Re: [HACKERS] Proposal: Generic WAL logical messages

On 22/03/16 12:47, Andres Freund wrote: On 2016-03-21 18:10:55 +0100, Petr Jelinek wrote: + + + Generic Message Callback + + + The optional message_cb callback is called whenever + a logical decoding message has been decoded. + +typedef void (*LogicalDecodeMessageCB) ( +

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-03-21 18:10:55 +0100, Petr Jelinek wrote: > Just noticed there is missing symlink in the pg_xlogdump. > create mode 100644 src/backend/access/rmgrdesc/logicalmsgdesc.c > create mode 12 src/bin/pg_xlogdump/logicalmsgdesc.c Uh, src/bin/pg_xlogdump/logicalmsgdesc.c shouldn't be there.

Re: [HACKERS] Proposal: Generic WAL logical messages

Just noticed there is missing symlink in the pg_xlogdump. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services >From f47730e5e8ef5797c7595aafcbf8cff3b375d0ad Mon Sep 17 00:00:00 2001 From: Petr Jelinek

Re: [HACKERS] Proposal: Generic WAL logical messages

On 18 March 2016 at 20:36, Artur Zakirov wrote: > On 17.03.2016 15:42, Craig Ringer wrote: > >> >> >> Would you mind sharing the plugin here? I could add it to >> src/test/modules and add some t/ tests so it runs under the TAP test >> framework. >> >> >> -- >> Craig

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, thanks for review. On 17/03/16 13:36, Tomas Vondra wrote: Hi, a few comments about the last version of the patch: 1) LogicalDecodeMessageCB Do we actually need the 'transactional' parameter here? I mean, having the 'txn' should be enough, as transactional = (txt != NULL)

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, a few comments about the last version of the patch: 1) LogicalDecodeMessageCB Do we actually need the 'transactional' parameter here? I mean, having the 'txn' should be enough, as transactional = (txt != NULL) Of course, having a simple flag is more convenient. 2)

Re: [HACKERS] Proposal: Generic WAL logical messages

On 17 March 2016 at 19:08, Artur Zakirov wrote: > On 16.03.2016 18:56, David Steele wrote: > >> >> This patch applies cleanly and is ready for review with no outstanding >> issues that I can see. Simon and Artur, you are both signed up as >> reviewers. Care to take a

Re: [HACKERS] Proposal: Generic WAL logical messages

On 16.03.2016 18:56, David Steele wrote: This patch applies cleanly and is ready for review with no outstanding issues that I can see. Simon and Artur, you are both signed up as reviewers. Care to take a crack at it? Thanks, I have tested the patch once again and have looked the code. It

Re: [HACKERS] Proposal: Generic WAL logical messages

On 3/9/16 6:58 PM, Petr Jelinek wrote: > On 08/03/16 21:21, Artur Zakirov wrote: >> I think here >> >>> +const char * >>> +logicalmsg_identify(uint8 info) >>> +{ >>> +if (info & ~XLR_INFO_MASK == XLOG_LOGICAL_MESSAGE) >>> +return "MESSAGE"; >>> + >>> +return NULL; >>> +} >> >> we

Re: [HACKERS] Proposal: Generic WAL logical messages

On 08/03/16 21:21, Artur Zakirov wrote: I think here +const char * +logicalmsg_identify(uint8 info) +{ +if (info & ~XLR_INFO_MASK == XLOG_LOGICAL_MESSAGE) +return "MESSAGE"; + +return NULL; +} we should use brackets const char * logicalmsg_identify(uint8 info) { if

Re: [HACKERS] Proposal: Generic WAL logical messages

I think here +const char * +logicalmsg_identify(uint8 info) +{ + if (info & ~XLR_INFO_MASK == XLOG_LOGICAL_MESSAGE) + return "MESSAGE"; + + return NULL; +} we should use brackets const char * logicalmsg_identify(uint8 info) { if ((info & ~XLR_INFO_MASK) ==

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, attached is the newest version of the patch. I removed the registry, renamed the 'send' to 'emit', documented the callback parameters properly. I also added the test to ddl.sql for the serialization and deserialization (and of course found a bug there) and in general fixed all the stuff

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, On 2016-02-28 22:44:12 +0100, Petr Jelinek wrote: > On 27/02/16 01:05, Andres Freund wrote: > >I'm not really convinced by RegisterStandbyMsgPrefix() et al. There's > >not much documentation about what it actually is supposed to > >acomplish. Afaics you're basically forced to use >

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, thanks for looking Andres, On 27/02/16 01:05, Andres Freund wrote: Hi, I'm not really convinced by RegisterStandbyMsgPrefix() et al. There's not much documentation about what it actually is supposed to acomplish. Afaics you're basically forced to use shared_preload_libraries with it

Re: [HACKERS] Proposal: Generic WAL logical messages

Hello, On 27.02.2016 03:05, Andres Freund wrote: Hi, I'm not really convinced by RegisterStandbyMsgPrefix() et al. There's not much documentation about what it actually is supposed to acomplish. Afaics you're basically forced to use shared_preload_libraries with it right now? Also, iterating

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, I'm not really convinced by RegisterStandbyMsgPrefix() et al. There's not much documentation about what it actually is supposed to acomplish. Afaics you're basically forced to use shared_preload_libraries with it right now? Also, iterating through a linked list everytime something is logged

Re: [HACKERS] Proposal: Generic WAL logical messages

Hello, It seems that you forgot regression tests for test_decoding. There is an entry in test_decoding/Makefile, but there are not files sql/messages.sql and expected/messages.out. However they are included in the first version of the patch. Hi, yes, git add missing. -- Petr Jelinek

Re: [HACKERS] Proposal: Generic WAL logical messages

On 23.01.2016 01:22, Petr Jelinek wrote: Hi, here is updated version of this patch, calling the messages logical (decoding) messages consistently everywhere and removing any connection to standby messages. Moving this to it's own module gave me place to write some brief explanation about this

Re: [HACKERS] Proposal: Generic WAL logical messages

On 1 February 2016 at 09:45, Alexander Korotkov wrote: > On Sat, Jan 30, 2016 at 11:58 AM, Simon Riggs wrote: >> >> On 29 January 2016 at 21:11, Alexander Korotkov >> wrote: >>> >>> Should we think more about naming?

Re: [HACKERS] Proposal: Generic WAL logical messages

On 29 January 2016 at 21:11, Alexander Korotkov wrote: > Hi, Petr! > > On Sat, Jan 23, 2016 at 1:22 AM, Petr Jelinek > wrote: > >> here is updated version of this patch, calling the messages logical >> (decoding) messages consistently everywhere

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, Petr! On Sat, Jan 23, 2016 at 1:22 AM, Petr Jelinek wrote: > here is updated version of this patch, calling the messages logical > (decoding) messages consistently everywhere and removing any connection to > standby messages. Moving this to it's own module gave me

Re: [HACKERS] Proposal: Generic WAL logical messages

Hi, here is updated version of this patch, calling the messages logical (decoding) messages consistently everywhere and removing any connection to standby messages. Moving this to it's own module gave me place to write some brief explanation about this so the code documentation has hopefully

Re: [HACKERS] Proposal: Generic WAL logical messages

On 2016-01-07 17:22, Simon Riggs wrote: * You call them "logical messages" here, but standby messages in code. But they only apply to logical decoding, so "logical message" seems a better name. Can we avoid calling them "messages" cos that will get confusing. Yes it's slightly confusing, the

Re: [HACKERS] Proposal: Generic WAL logical messages

> Process-wise, I don't understand why is Andres mentioned as co-author. > Did he actually wrote part of the patch, or advised on the design? > Who is reviewing the patch? It's extracted & extended from BDR, where I added that feature (to implement distributed locking). -- Sent via