Re: Perform streaming logical transactions by background workers and parallel apply

At Mon, 24 Apr 2023 10:55:44 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in
> While looking at the worker.c, I realized that we have the following
> code in handle_streamed_transaction():
>
> default:
> Assert(false);
> return false; / silence compiler warning /
>
> I think it's better to do elog(ERROR) instead of Assert() as it ends
> up returning false in non-assertion builds, which might cause a
> problem. And it's more consistent with other codes in worker.c. Please
> find an attached patch.

I concur that returning false is problematic.

For assertion builds, Assert typically provides more detailed
information than elog. However, in this case, it wouldn't matter much
since the worker would repeatedly restart even after a server-restart
for the same reason unless cosmic rays are involved. Moreover, the
situation doesn't justify server-restaring, as it would unnecessarily
involve other backends.

In my opinion, it is fine to replace the Assert with an ERROR.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center