Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-09-03 Thread Robert Haas 
On Fri, Aug 29, 2014 at 9:48 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
 Hi all,

 Using a plugin producing binary output, I came across this error:
 =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
 ERROR:  0A000: output plugin cannot produce binary output
 LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404

 Shouldn't the error message be here something like binary output plugin
 cannot produce textual output? The plugin used in my case produces binary
 output, but what is requested from it with pg_logical_slot_peek_changes is
 textual output.

 I don't like the new message much. It's imo even more misleading than
 the old message. How about
 output pluing produces binary output but the sink only accepts textual data?

Maybe:

ERROR: pg_logical_slot_peek_changes cannot be used with a plugin that
produces only binary output
HINT: Use pg_logical_slot_peek_binary_changes instead.

-- 
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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-09-03 Thread Andres Freund 
On 2014-09-03 09:36:32 -0400, Robert Haas wrote:
 On Fri, Aug 29, 2014 at 9:48 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
  Hi all,
 
  Using a plugin producing binary output, I came across this error:
  =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
  ERROR:  0A000: output plugin cannot produce binary output
  LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
 
  Shouldn't the error message be here something like binary output plugin
  cannot produce textual output? The plugin used in my case produces binary
  output, but what is requested from it with pg_logical_slot_peek_changes is
  textual output.
 
  I don't like the new message much. It's imo even more misleading than
  the old message. How about
  output pluing produces binary output but the sink only accepts textual 
  data?
 
 Maybe:
 
 ERROR: pg_logical_slot_peek_changes cannot be used with a plugin that
 produces only binary output
 HINT: Use pg_logical_slot_peek_binary_changes instead.

That level has no knowledge of what it's used by, so I think that'd
require bigger changes than worth it.

Greetings,

Andres Freund

-- 
 Andres Freund 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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-09-03 Thread Robert Haas 
On Wed, Sep 3, 2014 at 10:45 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-09-03 09:36:32 -0400, Robert Haas wrote:
 On Fri, Aug 29, 2014 at 9:48 AM, Andres Freund and...@2ndquadrant.com 
 wrote:
  On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
  Hi all,
 
  Using a plugin producing binary output, I came across this error:
  =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
  ERROR:  0A000: output plugin cannot produce binary output
  LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
 
  Shouldn't the error message be here something like binary output plugin
  cannot produce textual output? The plugin used in my case produces binary
  output, but what is requested from it with pg_logical_slot_peek_changes is
  textual output.
 
  I don't like the new message much. It's imo even more misleading than
  the old message. How about
  output pluing produces binary output but the sink only accepts textual 
  data?

 Maybe:

 ERROR: pg_logical_slot_peek_changes cannot be used with a plugin that
 produces only binary output
 HINT: Use pg_logical_slot_peek_binary_changes instead.

 That level has no knowledge of what it's used by, so I think that'd
 require bigger changes than worth it.

ERROR: this logical decoding plugin can only produce binary output
ERROR: logical decoding plugin %s can only produce binary output

?

-- 
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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-09-03 Thread Andres Freund 
On 2014-09-03 10:59:17 -0400, Robert Haas wrote:
 On Wed, Sep 3, 2014 at 10:45 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-09-03 09:36:32 -0400, Robert Haas wrote:
  On Fri, Aug 29, 2014 at 9:48 AM, Andres Freund and...@2ndquadrant.com 
  wrote:
   On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
   Hi all,
  
   Using a plugin producing binary output, I came across this error:
   =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
   ERROR:  0A000: output plugin cannot produce binary output
   LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
  
   Shouldn't the error message be here something like binary output plugin
   cannot produce textual output? The plugin used in my case produces 
   binary
   output, but what is requested from it with pg_logical_slot_peek_changes 
   is
   textual output.
  
   I don't like the new message much. It's imo even more misleading than
   the old message. How about
   output pluing produces binary output but the sink only accepts textual 
   data?
 
  Maybe:
 
  ERROR: pg_logical_slot_peek_changes cannot be used with a plugin that
  produces only binary output
  HINT: Use pg_logical_slot_peek_binary_changes instead.
 
  That level has no knowledge of what it's used by, so I think that'd
  require bigger changes than worth it.
 
 ERROR: this logical decoding plugin can only produce binary output
 ERROR: logical decoding plugin %s can only produce binary output

ERROR: logical decoding plugin %s produces binary output, but sink only copes 
with textual data

Not sure about 'sink'. Maybe 'receiving side' or 'receiver'?

Not 100% sure if the name is available in that site, but if not it can
be left of without hurting much.

Greetings,

Andres Freund

-- 
 Andres Freund 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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-09-03 Thread Robert Haas 
On Wed, Sep 3, 2014 at 11:06 AM, Andres Freund and...@2ndquadrant.com wrote:
  Maybe:
 
  ERROR: pg_logical_slot_peek_changes cannot be used with a plugin that
  produces only binary output
  HINT: Use pg_logical_slot_peek_binary_changes instead.
 
  That level has no knowledge of what it's used by, so I think that'd
  require bigger changes than worth it.

 ERROR: this logical decoding plugin can only produce binary output
 ERROR: logical decoding plugin %s can only produce binary output

 ERROR: logical decoding plugin %s produces binary output, but sink only 
 copes with textual data

 Not sure about 'sink'. Maybe 'receiving side' or 'receiver'?

 Not 100% sure if the name is available in that site, but if not it can
 be left of without hurting much.

I was trying to avoid mentioning the word sink because we don't
actually have a real term for that.  From the user's perspective, it's
not going to be obvious that the function they invoked is the sink or
receiver; to them, it's just an interface - if anything, it's a
*sender* of the changes to them.

In case I lose that argument, please at least write allows instead
of copes with; the latter I think is too informal for an error
message.

-- 
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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-09-03 Thread Andres Freund 
On 2014-09-03 11:23:21 -0400, Robert Haas wrote:
 On Wed, Sep 3, 2014 at 11:06 AM, Andres Freund and...@2ndquadrant.com wrote:
   Maybe:
  
   ERROR: pg_logical_slot_peek_changes cannot be used with a plugin that
   produces only binary output
   HINT: Use pg_logical_slot_peek_binary_changes instead.
  
   That level has no knowledge of what it's used by, so I think that'd
   require bigger changes than worth it.
 
  ERROR: this logical decoding plugin can only produce binary output
  ERROR: logical decoding plugin %s can only produce binary output
 
  ERROR: logical decoding plugin %s produces binary output, but sink only 
  copes with textual data
 
  Not sure about 'sink'. Maybe 'receiving side' or 'receiver'?
 
  Not 100% sure if the name is available in that site, but if not it can
  be left of without hurting much.
 
 I was trying to avoid mentioning the word sink because we don't
 actually have a real term for that.

I understand the hesitation. I don't like it either, but I don't think
it gets clearer by leaving it off entirely.

  From the user's perspective, it's
 not going to be obvious that the function they invoked is the sink or
 receiver; to them, it's just an interface - if anything, it's a
 *sender* of the changes to them.

Is 'logical output method' perhaps better? It'd coincide with the terms
in the code and docs too.

 In case I lose that argument, please at least write allows instead
 of copes with; the latter I think is too informal for an error
 message.

Ok, sure.

Greetings,

Andres Freund

-- 
 Andres Freund 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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-30 Thread Michael Paquier 
On Fri, Aug 29, 2014 at 11:15 PM, Andres Freund and...@2ndquadrant.com
wrote:

  On top of that, a
  logical receiver receives data in textual form even if the decoder is
  marked as binary when getting a message with PQgetCopyData.

 I have no idea what you mean by that. The data is sent in the format the
 output plugin writes the data in.


Well, that's where we don't understand each other. By reading the docs I
understand that by setting output_type to OUTPUT_PLUGIN_BINARY_OUTPUT, all
the changes generated by an output plugin would be automatically converted
to binary and sent to a client back as binary data, but that's not the
case. For example, when using pg_recvlogical on a slot plugged with
test_decoding, the output received is not changed and remains like that
even when using force-binary:
BEGIN 1000
table public.aa: INSERT: a[integer]:2
COMMIT 1000
Perhaps I didn't understand the docs well, but this is confusing and it
should be clearly specified to the user that output_type only impacts the
SQL interface with the peekget functions (as far as I tested). As things
stand now, by reading the docs a user can only know that output_type can be
set as OUTPUT_PLUGIN_BINARY_OUTPUT or OUTPUT_PLUGIN_TEXTUAL_OUTPUT, but
there is nothing about what each value means and how it impacts the output
format, and you need to look at the source code to get that this parameter
is used for some sanity checks, only within the logical functions. That's
not really user-friendly.

Attached is a patch improving the documentation regarding those comments.
Regards,
-- 
Michael
diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml
index e6880d0..1472a6c 100644
--- a/doc/src/sgml/logicaldecoding.sgml
+++ b/doc/src/sgml/logicaldecoding.sgml
@@ -403,9 +403,17 @@ typedef struct OutputPluginOptions
 OutputPluginOutputType output_type;
 } OutputPluginOptions;
 /programlisting
-  literaloutput_type/literal has to either be set to
-  literalOUTPUT_PLUGIN_TEXTUAL_OUTPUT/literal
-  or literalOUTPUT_PLUGIN_BINARY_OUTPUT/literal.
+  literaloutput_type/literal is the parameter defining the output
+  format of plugin; it can be one of
+  literalOUTPUT_PLUGIN_TEXTUAL_OUTPUT/literal (support for output
+  format as text) and literalOUTPUT_PLUGIN_BINARY_OUTPUT/literal
+  (support for format output as binary). This parameter influences only
+  the way output changes can be accessed by the set of SQL functions able
+  to query a replication slot. For example, the changes fetched by
+  xref linkend=app-pgrecvlogical are not impacted by this parameter
+  and remain the same, while functionpg_logical_slot_get_changes/
+  is not able to query changes using output type
+  literalOUTPUT_PLUGIN_BINARY_OUTPUT/literal.
  /para
  para
   The startup callback should validate the options present in
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index a3a58e6..4f802ad 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -394,14 +394,14 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 		MemoryContextSwitchTo(oldcontext);
 
 		/*
-		 * Check whether the output pluggin writes textual output if that's
+		 * Check whether the output plugin writes textual output if that's
 		 * what we need.
 		 */
 		if (!binary 
 			ctx-options.output_type != OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
 			ereport(ERROR,
 	(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-	 errmsg(output plugin cannot produce binary output)));
+	 errmsg(output plugin produces binary output but only textual output is accepted)))
 
 		ctx-output_writer_private = p;
 
diff --git a/src/include/catalog/objectaccess.h b/src/include/catalog/objectaccess.h
index 4fdd056..1e1cb13 100644
--- a/src/include/catalog/objectaccess.h
+++ b/src/include/catalog/objectaccess.h
@@ -13,7 +13,7 @@
 /*
  * Object access hooks are intended to be called just before or just after
  * performing certain actions on a SQL object.  This is intended as
- * infrastructure for security or logging pluggins.
+ * infrastructure for security or logging plugins.
  *
  * OAT_POST_CREATE should be invoked just after the object is created.
  * Typically, this is done after inserting the primary catalog records and

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

[HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Michael Paquier 
Hi all,

Using a plugin producing binary output, I came across this error:
=# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
ERROR:  0A000: output plugin cannot produce binary output
LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404

Shouldn't the error message be here something like binary output plugin
cannot produce textual output? The plugin used in my case produces binary
output, but what is requested from it with pg_logical_slot_peek_changes is
textual output.

A patch is attached (with s/pluggin/plugin in bonus). Comments welcome.
Regards,
-- 
Michael
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index a3a58e6..310b1e5 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -394,14 +394,14 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 		MemoryContextSwitchTo(oldcontext);
 
 		/*
-		 * Check whether the output pluggin writes textual output if that's
+		 * Check whether the output plugin writes textual output if that's
 		 * what we need.
 		 */
 		if (!binary 
 			ctx-options.output_type != OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
 			ereport(ERROR,
 	(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-	 errmsg(output plugin cannot produce binary output)));
+	 errmsg(binary output plugin cannot produce textual output)));
 
 		ctx-output_writer_private = p;
 
diff --git a/src/include/catalog/objectaccess.h b/src/include/catalog/objectaccess.h
index 4fdd056..1e1cb13 100644
--- a/src/include/catalog/objectaccess.h
+++ b/src/include/catalog/objectaccess.h
@@ -13,7 +13,7 @@
 /*
  * Object access hooks are intended to be called just before or just after
  * performing certain actions on a SQL object.  This is intended as
- * infrastructure for security or logging pluggins.
+ * infrastructure for security or logging plugins.
  *
  * OAT_POST_CREATE should be invoked just after the object is created.
  * Typically, this is done after inserting the primary catalog records and

-- 
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] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Andres Freund 
On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
 Hi all,
 
 Using a plugin producing binary output, I came across this error:
 =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
 ERROR:  0A000: output plugin cannot produce binary output
 LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
 
 Shouldn't the error message be here something like binary output plugin
 cannot produce textual output? The plugin used in my case produces binary
 output, but what is requested from it with pg_logical_slot_peek_changes is
 textual output.

I don't like the new message much. It's imo even more misleading than
the old message. How about
output pluing produces binary output but the sink only accepts textual data?

Greetings,

Andres Freund

-- 
 Andres Freund 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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Michael Paquier 
On Fri, Aug 29, 2014 at 10:48 PM, Andres Freund and...@2ndquadrant.com
wrote:

 On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
  Hi all,
 
  Using a plugin producing binary output, I came across this error:
  =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
  ERROR:  0A000: output plugin cannot produce binary output
  LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
 
  Shouldn't the error message be here something like binary output plugin
  cannot produce textual output? The plugin used in my case produces
 binary
  output, but what is requested from it with pg_logical_slot_peek_changes
 is
  textual output.

 I don't like the new message much. It's imo even more misleading than
 the old message. How about
 output plugin produces binary output but the sink only accepts textual
 data?

Sounds fine for me. I am sure that native English speakers will come up as
well with additional suggestions.

Btw, I am having a hard time understanding in which case
OUTPUT_PLUGIN_BINARY_OUTPUT is actually useful. Looking at the code it is
only a subset of what OUTPUT_PLUGIN_TEXTUAL_OUTPUT can do as textual can
generate both binary and textual output, while binary can only generate
binary output. The only code path where a flag OUTPUT_PLUGIN_* is used is
in this code path for this very specific error message. On top of that, a
logical receiver receives data in textual form even if the decoder is
marked as binary when getting a message with PQgetCopyData.

Perhaps I am missing something... But in this case I think that the
documentation should have a more detailed explanation about the output
format options. Currently only the option value is mentioned, and there is
nothing about what they actually do. This is error-prone for the user.
Regards,
-- 
Michael

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Andres Freund 
On 2014-08-29 23:07:44 +0900, Michael Paquier wrote:
 On Fri, Aug 29, 2014 at 10:48 PM, Andres Freund and...@2ndquadrant.com
 wrote:
 
  On 2014-08-29 22:42:46 +0900, Michael Paquier wrote:
   Hi all,
  
   Using a plugin producing binary output, I came across this error:
   =# select data from pg_logical_slot_peek_changes('foo', NULL, NULL);
   ERROR:  0A000: output plugin cannot produce binary output
   LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
  
   Shouldn't the error message be here something like binary output plugin
   cannot produce textual output? The plugin used in my case produces
  binary
   output, but what is requested from it with pg_logical_slot_peek_changes
  is
   textual output.
 
  I don't like the new message much. It's imo even more misleading than
  the old message. How about
  output plugin produces binary output but the sink only accepts textual
  data?
 
 Sounds fine for me. I am sure that native English speakers will come up as
 well with additional suggestions.
 
 Btw, I am having a hard time understanding in which case
 OUTPUT_PLUGIN_BINARY_OUTPUT is actually useful. Looking at the code it is
 only a subset of what OUTPUT_PLUGIN_TEXTUAL_OUTPUT can do as textual can
 generate both binary and textual output, while binary can only generate
 binary output.

No, a textual output plugin is *NOT* allowed to produce binary
output. That'd violate e.g. pg_logical_slot_peek_changes's return type
because it's only declared to return text.
If you compile with assertions enabled not adhering to that will
actually result in an error:
/*
 * Assert ctx-out is in database encoding when we're writing textual
 * output.
 */
if (!p-binary_output)
Assert(pg_verify_mbstr(GetDatabaseEncoding(),
   ctx-out-data, 
ctx-out-len,
   false));

 The only code path where a flag OUTPUT_PLUGIN_* is used is
 in this code path for this very specific error message.

Well, LogicalDecodingContext-binary_output is checked too.

 On top of that, a
 logical receiver receives data in textual form even if the decoder is
 marked as binary when getting a message with PQgetCopyData.

I have no idea what you mean by that. The data is sent in the format the
output plugin writes the data in.

Greetings,

Andres Freund

-- 
 Andres Freund 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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Michael Paquier 
On Fri, Aug 29, 2014 at 11:15 PM, Andres Freund and...@2ndquadrant.com
wrote:

 No, a textual output plugin is *NOT* allowed to produce binary
 output. That'd violate e.g. pg_logical_slot_peek_changes's return type
 because it's only declared to return text.


A textual output plugin can call pg_logical_slot_peek_binary_changes and
pg_logical_slot_peek_changes as well,
and a binary output plugin can only call
pg_logical_slot_peek_binary_changes, and will error out with
pg_logical_slot_peek_changes:
=# select pg_create_logical_replication_slot('foo', 'test_decoding');
 pg_create_logical_replication_slot

 (foo,0/16C6880)
(1 row)
=# create table aa as select 1;
SELECT 1
=# select substring(encode(data, 'escape'), 1, 20),
   substring(data, 1, 20)
 FROM pg_logical_slot_peek_binary_changes('foo', NULL, NULL);
  substring   | substring
--+
 BEGIN 1000   | \x424547494e2031303030
 table public.aa: INS | \x7461626c65207075626c69632e61613a20494e53
 COMMIT 1000  | \x434f4d4d49542031303030
(3 rows)
=# select pg_logical_slot_peek_changes('foo', NULL, NULL, 'force-binary',
'true');
ERROR:  0A000: output plugin cannot produce binary output
LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
=# select substring(data, 1, 20)
from pg_logical_slot_peek_binary_changes('foo', NULL, NULL,
'force-binary', 'true');
 substring

 \x424547494e2031303030
 \x7461626c65207075626c69632e61613a20494e53
 \x434f4d4d49542031303030
(3 rows)

Is that expected?
-- 
Michael

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Andres Freund 
On 2014-08-29 23:31:49 +0900, Michael Paquier wrote:
 On Fri, Aug 29, 2014 at 11:15 PM, Andres Freund and...@2ndquadrant.com
 wrote:
 
  No, a textual output plugin is *NOT* allowed to produce binary
  output. That'd violate e.g. pg_logical_slot_peek_changes's return type
  because it's only declared to return text.
 
 
 A textual output plugin can call pg_logical_slot_peek_binary_changes and
 pg_logical_slot_peek_changes as well,

Well, for one a output plugin doesn't call
pg_logical_slot_peek_binary_changes, it's the other way round. And sure:
Every text is also binary. So that's just fine.

 and a binary output plugin can only call
 pg_logical_slot_peek_binary_changes, and will error out with
 pg_logical_slot_peek_changes:
 =# select pg_create_logical_replication_slot('foo', 'test_decoding');
  pg_create_logical_replication_slot
 
  (foo,0/16C6880)
 (1 row)
 =# create table aa as select 1;
 SELECT 1
 =# select substring(encode(data, 'escape'), 1, 20),
substring(data, 1, 20)
  FROM pg_logical_slot_peek_binary_changes('foo', NULL, NULL);
   substring   | substring
 --+
  BEGIN 1000   | \x424547494e2031303030
  table public.aa: INS | \x7461626c65207075626c69632e61613a20494e53
  COMMIT 1000  | \x434f4d4d49542031303030
 (3 rows)
 =# select pg_logical_slot_peek_changes('foo', NULL, NULL, 'force-binary',
 'true');
 ERROR:  0A000: output plugin cannot produce binary output
 LOCATION:  pg_logical_slot_get_changes_guts, logicalfuncs.c:404
 =# select substring(data, 1, 20)
 from pg_logical_slot_peek_binary_changes('foo', NULL, NULL,
 'force-binary', 'true');
  substring
 
  \x424547494e2031303030
  \x7461626c65207075626c69632e61613a20494e53
  \x434f4d4d49542031303030
 (3 rows)
 
 Is that expected?

Yes. The output plugin declares whether it requires the *output method*
to support binary data. pg_logical_slot_peek_changes *can not* support
binary data because it outputs data as
text. pg_logical_slot_peek_binary_changes *can* support binary data
because it returns bytea (and thus it also can output text, because
that's essentially a subset of binary data).

Greetings,

Andres Freund

-- 
 Andres Freund 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

Re: [HACKERS] Misleading error message in logical decoding for binary plugins

2014-08-29 Thread Michael Paquier 
On Fri, Aug 29, 2014 at 11:39 PM, Andres Freund and...@2ndquadrant.com
wrote:

 Yes. The output plugin declares whether it requires the *output method*
 to support binary data. pg_logical_slot_peek_changes *can not* support
 binary data because it outputs data as
 text. pg_logical_slot_peek_binary_changes *can* support binary data
 because it returns bytea (and thus it also can output text, because
 that's essentially a subset of binary data).

Thanks for the explanations. This would meritate more details within the
docs, like what those two modes actually do and what the user can expect as
differences, advantages and disadvantages if he chooses one or the other
when starting decoding...
-- 
Michael