Re: Inconsistent error handling in START_REPLICATION command

On Fri, Mar 11, 2016 at 11:36 AM, David Steele <david(at)pgmasters(dot)net> wrote:
> It looks like a decision needs to be made here whether to apply this patch,
> send it back to the author, or reject it so I'm marking it "Ready for
> Committer".
>
> Robert, since you were participating in this conversation can you have a
> look?

Who, me? *looks around*

OK, I had a look. I don't think this is a bug. The original report
is complaining about this:

=================
psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0 ("invalid" 'value');
unexpected PQresultStatus: 8

The last command results in the following output sent to the server log:

ERROR: option "invalid" = "value" is unknown
CONTEXT: slot "test1", output plugin "test_decoding", in the startup callback
=================

But that's just because psql knows utterly zero about copy-both mode.
So the problem here isn't that the server is doing anything it
shouldn't be doing; the server code is totally legitimate. It's just
that the client is stupid.

Now, we could commit this patch anyway. It won't hurt anything. But
it won't help very much, either. The fundamental issue is that
issuing a START_REPLICATION_SLOT command in psql can't be expected to
work. If you do that, it's either going to fail with a server error,
or it's going to fail with "unexpected PQresultStatus: 8". Committing
this patch would cause this particular example to fail in the first
way rather than the second one, and I'm not direly opposed to that if
somebody really feels strongly about it. But it seems like the wrong
thing to focus on. I think the real question is: why are you
insisting on issuing a command in psql that can NEVER succeed there?
If you want that to work, you're going to need to add copy-both
support to psql, which might be a fun project but has nothing to do
with this patch.

So I am inclined to think it's fine to reject this, unless somebody
wants to argue that there's some advantage to changing it than I am
missing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company