Re: walsender & parallelism

Hi,

On 2017-04-24 07:31:18 +0200, Petr Jelinek wrote:
> The previous coding tried to run the unknown string throur lexer which
> could fail for some valid SQL statements as the replication command
> parser is too simple to handle all the complexities of SQL language.
>
> Instead just fall back to SQL parser on any unknown command.
>
> Also remove SHOW command handling from walsender as it's handled by the
> simple query code path.

This break SHOW var; over the plain replication connections though
(which was quite intentionally supported), so I don't think that's ok?

I don't like much how SHOW and walsender understanding SQL statements
turned out, I think code like

if (am_walsender)
{
if (!exec_replication_command(query_string))
exec_simple_query(query_string);
}
else
exec_simple_query(query_string);

shows some of the issues here.

> Checks the SQL over walsender interface by running the standard set of
> regression tests against it.

I like that approach a lot.

> New alternative output for largeobject test has been added as the
> replication connection does not support fastpath function calls.

I think just supporting fastpath over the replication protocol is less
work than maintaining the alternative file.

> --- a/src/Makefile.global.in
> +++ b/src/Makefile.global.in
> @@ -321,6 +321,7 @@ BZIP2 = bzip2
> # Testing
>
> check: temp-install
> +check-walsender-sql: temp-install

This doesn't strike me as something that should go into
a/src/Makefile.global.in - I'd rather put it in
b/src/test/regress/GNUmakefile. check is in .global because it's used
in a lot of different makefiles, but that's not true for this target, right?

Greetings,

Andres Freund