Re: Misleading error message in logical decoding for binary plugins

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