Re: Misleading error message in logical decoding for binary plugins

On 2014-08-29 23:07:44 +0900, Michael Paquier wrote:
> On Fri, Aug 29, 2014 at 10:48 PM, Andres Freund <andres(at)2ndquadrant(dot)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