Re: Removing support for FE/BE protocol version 2

Hi Tatsuo,

Thank you for v3!

I applied both patches on master. The build is clean
(no new warnings) and the full regression suite passes on PG16. I
checked the bytes on the wire: 'E' + "FATAL: unsupported frontend
protocol 2.0\n" + '\0'. Looks good.

0001:

- src/protocol/pool_proto_modules.c, ProcessBackendResponse(): when
the "if (MAJOR(backend) == PROTO_MAJOR_V3) { switch ... }" was
flattened, the closing brace of the switch was lost. Now

default:
status = SimpleForwardToFrontend(kind, frontend,
backend);
break;

pool_get_session_context(false)->flush_pending = false;
}

so the flush_pending reset is unreachable. In master it ran after
every backend message, and pool_process_query.c reads flush_pending.
The regression tests pass anyway, so nothing seems to cover it.

- pool_proto_modules.h still declares the functions that were in
pool_proto2.c: AsciiRow, BinaryRow, CompletedResponse,
CursorResponse, EmptyQueryResponse, FunctionResultResponse,
NotificationResponse, RowDescription, ErrorResponse, NoticeResponse.
The file header comment still mentions pool_proto2.c too.

- More v2-only code that is now dead:
- FunctionCall() in pool_proto_modules.c (FunctionCall3() is used
instead)
- the tail of CopyDataRows() after the copyin/copyout branches (the
"string == NULL" check, the pool_write() calls, and the "\." check)
- pool_read_string() in pool_stream.c (no callers left)
- the num_fields argument of ProcessBackendResponse() (unused now;
the comment in pool_process_query.c still says "V2 protocol")

- StartupPacket_v2 is still in pool.h and typedefs.list.

- The do_query() stub in
src/test/regression/tests/010.rewrite_timestamp/timestamp/main.c
still has the old 4-argument signature.

- Some MAJOR(backend) checks are now always true: child.c
("MAJOR(backend) == 3", twice), pool_query_context.c (twice) and
pool_memqcache.c. You want to keep the protoMajor plumbing for a
possible v4, so maybe these should stay too. Just mentioning them.

- elog.c: trailing whitespace in the new comment ("protocol ").

0002:

- write_it() is still not the original: it is now public and calls
exit(1) on failure. If connect_v2() needs the exit, maybe add a
separate helper and keep write_it() as it was.

- connect_v2(): if read() of the message kind fails, we print an error
but continue and use the uninitialized kind. I think it should exit.
Also a typo: "canot".

- read_string_internal(): after pg_realloc(), p still points into the
old buffer, so strings longer than 512 bytes break. This bug was
already in read_string(), but now connect_v2() uses it too. Something
like "p = buf + (PROTO_ALLOC_SIZE * (alloc_factor - 1));" after the
realloc should fix it.

- Minor: the code now uses %m in fprintf() in several places (main.c,
read.c, send.c). That is glibc-only; on macOS it prints a literal
"m". The original code used strerror(errno).

Best,

On Sat, Oct 10, 2026 at 10:51 AM Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:

> Hi Nadav,
>
> Thank you for the review!
>
> > A few things i noticed reviewing the changes.
> >
> >> Since we cannot send v3 error response message to v2 frontend, I end
> >> up with the code below:
> >>
> >> + case PROTO_MAJOR_V2: /* V2 protocol is not supported */
> >> + ereport(FATAL,
> >> + (errmsg("FATAL: unsupported
> frontend
> > protocol %u.%u",
> >> + sp->major,
> > sp->minor)));
> >
> > I think there is a problem in the v2 branch of
> > send_message_to_frontend() in elog.c:
> >
> > pool_send_to_frontend((edata->elevel < ERROR) ? "N" : "E", 1,
> false);
> > pool_send_to_frontend(message, strlen(message), false);
> > pool_send_to_frontend("\n", 1, true);
> >
> > A v2 ErrorResponse is 'E' followed by a null-terminated string, but the
> > terminating '\0' is never sent. A real v2 client (libpq) reads until
> > the '\0', so it would probably get EOF and report "server closed the
> > connection unexpectedly" instead of our message. PostgreSQL's old-style
> > path includes the terminator (pq_putmessage_v2(..., buf.len + 1)).
> >
> > PostgreSQL also adds the severity prefix there ("%s: ",
> > error_severity(edata->elevel)) rather than in the message. If we do
> > the same in elog.c, the errmsg() can be just "unsupported frontend
> > protocol %u.%u". With "FATAL: " in errmsg(), I think pgpool's own log
> > would show "FATAL: FATAL: unsupported frontend protocol 2.0".
>
> Agreed. I fixed send_message_to_frontend() in elog.c. I mostly copy
> the v2 branch in PostgreSQL's send_message_to_frontend()).
>
> > Thanks! Some comments:
> >
> > - connect_v2() reads up to 512 bytes until EOF and prints them, so the
> > test passes even without the '\0' above. Could it read up to the
> > '\0' like libpq does, and fail if EOF comes first? Then the test
> > would cover this. Also, if the reply is 512 bytes or more, p is not
> > null-terminated before the fprintf("%s").
>
> Yes, we could. I added read_string_internal(), which accepts int file
> descriptor, rather than PQConn, and let connect_v2() call it for this
> purpose.
>
> > - connect_v2() always uses "/tmp" as the socket directory and ignores
> > -h when it starts with '/'. regress.sh lets you change
> > PGSOCKET_DIR (-s), so the test fails when /tmp is not used. Maybe
> > use -h as the socket directory like libpq does, and pass it from
> > test.sh.
>
> I modified connect_v2() to recognize -h when it starts with '/'. I
> also modified test.sh to give "-h localhost" to pgproto so that the
> test script does not depend on the socket directory.
>
> > - The new -t/--test-v2 option should be documented in
> > doc/src/sgml/ref/pgproto.sgml.
> >
> > - The PostgreSQL >= 14 and pgpool >= 4.8 checks in test.sh don't seem
> > necessary: pgpool rejects the packet before any backend sees it, and
> > the test always runs with pgpool from the same tree.
>
> Ok. The check removed.
>
> > - write_it() now exits on failure instead of printing a warning.
> > That's probably fine, but it changes behavior for the existing
> > pgproto tests.
>
> I undo the modification to write_it().
>
> > - Minor: read.c's copyright still says 2018-2023.
>
> Fixed.
>
> Attached are the v3 patch which addresses above.
>
> Regards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
>

--
Nadav Shatz
Tailor Brands | CTO