On 2014-09-23 12:40:25 +0900, Michael Paquier wrote:
> On Tue, Sep 23, 2014 at 4:59 AM, Robert Haas <robertmh...@gmail.com> wrote:
> > On Thu, Sep 18, 2014 at 11:21 AM, Michael Paquier
> > <michael.paqu...@gmail.com> wrote:
> >> On Thu, Sep 18, 2014 at 9:56 AM, Andres Freund <and...@2ndquadrant.com> 
> >> wrote:
> >>> On 2014-09-18 09:50:38 -0500, Michael Paquier wrote:
> >>>> > Do you see the difference between what your doc patch states and the
> >>>> > explanation I've given nearby in this thread?
> >>>> Perhaps that's the lack of documentation...
> >>>
> >>> Man. I've explained it to you about three times. The previous attempts
> >>> at doing so didn't seem to help. If my explanations don't explain it so
> >>> you can understand it adding them to the docs won't change a thing.
> >>> That's why I ask whether you see the difference?
> >> Urg sorry for the misunderstanding. The patch stated that this
> >> parameter only influences the output of the SQL functions while it
> >> defines if "the output plugin requires the output method to support
> >> binary data"?
> >
> > I'm not sure exactly what that sentence means.
> >
> > But here's the point: every series of bytes is a valid bytea, except
> > maybe if it's over 1GB and runs afould of MaxAllocSize.  But a series
> > of bytes is only a valid text datum if it's a valid sequence of
> > characters according to the database encoding.  We like to think of
> > text as being an arbitrary series of bytes, but it isn't.  It can't
> > contain any \0 bytes, and it can't contain anything that's invalid in
> > the database encoding.  bytea isn't subject to either of those
> > restrictions.
> >
> > So if we were going to have one universal output format for output
> > plugins, it would have to be bytea because that, really, can be
> > anything.  We could make users convert from that to text or whatever
> > they like.  But that's unappealing for several reasons: bytea output
> > is printed as unreadable hexademical garbage, and encoding conversions
> > are expensive.  So what we do instead is provide a text output method
> > and a binary output method.  That way, plugins that want to return
> > binary data are free to do so, and output methods that are happy to
> > return text can *declare* that what they return is legal text - and
> > then we just assume that to be true, and need not do an encoding
> > conversion.
> Aha, thanks. That's all clear then!

What about the attached patch then?

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 7722890556d1ce8a506d800720534448b6a97104 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 29 Sep 2014 12:48:39 +0200
Subject: [PATCH] Improve document about binary/textual output mode for output
 plugins.

Discussion: cab7npqqrqfzjqcjxu4gzztrd9kpj6hmn9g5aoomwt1wz8nf...@mail.gmail.com,
    CAB7nPqQXc_+g95zWnqaa=mvq4d3bvrs6t41frceyi2ocurr...@mail.gmail.com

Per discussion between Michael Paquier, Robert Haas and Andres Freund
---
 contrib/test_decoding/expected/binary.out      |  2 +-
 doc/src/sgml/func.sgml                         | 12 +++++++-----
 doc/src/sgml/logicaldecoding.sgml              | 20 +++++++++++++++++++-
 src/backend/replication/logical/logicalfuncs.c |  4 +++-
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/contrib/test_decoding/expected/binary.out b/contrib/test_decoding/expected/binary.out
index 4164784..6d30749 100644
--- a/contrib/test_decoding/expected/binary.out
+++ b/contrib/test_decoding/expected/binary.out
@@ -14,7 +14,7 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'for
 
 -- fails, binary plugin, textual consumer
 SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'force-binary', '1');
-ERROR:  output plugin cannot produce binary output
+ERROR:  logical decoding output plugin "test_decoding" produces binary output, but "pg_logical_slot_get_changes(name,pg_lsn,integer,text[])" expects textual data
 -- succeeds, textual plugin, binary consumer
 SELECT data FROM pg_logical_slot_get_binary_changes('regression_slot', NULL, NULL, 'force-binary', '0');
  data 
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7195df8..98cd02f 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16705,6 +16705,8 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
         the specified value.  Note, however, that the actual number of
         rows returned may be larger, since this limit is only checked after
         adding the rows produced when decoding each new transaction commit.
+        Can only be used on slots using a output plugin supporting textual
+        output.
        </entry>
       </row>
 
@@ -16739,7 +16741,8 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
        <entry>
         Behaves just like
         the <function>pg_logical_slot_get_changes()</function> function,
-        except that changes are returned as <type>bytea</type>.
+        except that changes are returned as <type>bytea</type> and that it can
+        be used on slots using output plugins that only support binary output.
        </entry>
       </row>
 
@@ -16755,10 +16758,9 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
        </entry>
        <entry>
         Behaves just like
-        the <function>pg_logical_slot_get_changes()</function> function,
-        except that changes are returned as <type>bytea</type> and that
-        changes are not consumed; that is, they will be returned again
-        on future calls.
+        the <function>pg_logical_slot_get_binary_changes()</function>
+        function, except that changes are not consumed; that is, they will be
+        returned again on future calls.
        </entry>
       </row>
      </tbody>
diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml
index e6880d0..0d9b76c 100644
--- a/doc/src/sgml/logicaldecoding.sgml
+++ b/doc/src/sgml/logicaldecoding.sgml
@@ -355,6 +355,23 @@ CREATE TABLE another_catalog_table(data text) WITH (user_catalog_table = true);
     </para>
    </sect2>
 
+   <sect2 id="logicaldecoding-output-mode">
+    <title>Output Modes</title>
+    <para>
+     Output plugin callbacks can pass data to the consumer in nearly arbitrary
+     formats. For some use cases, like viewing the changes via SQL, returning
+     data in a datatype that can contain arbitrary data (i.e. bytea) is
+     cumbersome. If the output plugin only outputs textual data in the
+     server's encoding it can declare that by
+     setting <literal>OutputPluginOptions.output_mode</>
+     to <literal>OUTPUT_PLUGIN_TEXTUAL_OUTPUT</> instead
+     of <literal>OUTPUT_PLUGIN_BINARY_OUTPUT</> in
+     the <link linkend="logicaldecoding-output-plugin-startup">startup
+     callback</>. In that case all the data has to be in the server's encoding
+     so a <type>text</> can contain it. This is checked in assertion enabled
+     builds.
+    </para>
+   </sect2>
    <sect2 id="logicaldecoding-output-plugin-callbacks">
     <title>Output Plugin Callbacks</title>
     <para>
@@ -405,7 +422,8 @@ typedef struct OutputPluginOptions
 </programlisting>
       <literal>output_type</literal> has to either be set to
       <literal>OUTPUT_PLUGIN_TEXTUAL_OUTPUT</literal>
-      or <literal>OUTPUT_PLUGIN_BINARY_OUTPUT</literal>.
+      or <literal>OUTPUT_PLUGIN_BINARY_OUTPUT</literal>. See also
+      <xref linkend="logicaldecoding-output-mode"/>.
      </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 9692f98..3a5ec2f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -401,7 +401,9 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 			ctx->options.output_type != OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("output plugin cannot produce binary output")));
+					 errmsg("logical decoding output plugin \"%s\" produces binary output, but \"%s\" expects textual data",
+							NameStr(MyReplicationSlot->data.plugin),
+							format_procedure(fcinfo->flinfo->fn_oid))));
 
 		ctx->output_writer_private = p;
 
-- 
1.8.3.251.g1462b67

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

Reply via email to