Re: Move global variables of pgoutput to plugin private scope.

Hi Hou-san.

Given there are some issues raised about the 0001 patch [1] I am
skipping that one until I see the replies.

Meanwhile, here are some review comments for the patches v1-0002 and v1-0003

////////////////////
v1-0002

======
Commit message

1.
The pgoutput module uses a global variable(publish_no_origin) to cache the
action for the origin filter. But we only initialize publish_no_origin when
user specifies the "origin" in the output paramters which means we could refer
to an uninitialized variable if user didn't specify the paramter.

~

1a.

typos
/variable(publish_no_origin)/variable (publish_no_origin)/
/paramters/parameters/
/paramter./paramter./

~

1b.
"...we could refer to an uninitialized variable"

I'm not sure what this means. Previously it was static, so it wouldn't
be "uninitialised"; it would be false. Perhaps there might be a stale
value from a previous pgoutput, but IIUC that's the point made by your
next paragraph ("Besides, we don't...")

~~~

2.
To improve it, the patch stores the map within the private data of the output
plugin so that it will get initialized and reset along with the output plugin
context.

2a.
/To improve it,/To fix this/

~

2b.
"stores the map"

What map? This might be a cut/paste error from the v1-0001 patch comment.

////////////////////
v1-0003

======
Commit message

1.
Missing patch comment.

======
src/backend/replication/pgoutput/pgoutput.c

2. maybe_send_schema

- if (in_streaming)
+ if (data->in_streaming)
set_schema_sent_in_streamed_txn((PGOutputData *) ctx->output_plugin_private,
relentry, topxid);
~

Since you added a new 'data' variable, you might as well make use of
it here instead of doing "(PGOutputData *) ctx->output_plugin_private"
again.

======
src/include/replication/pgoutput.h

3.
MemoryContext cachectx; /* private memory context for cache data */

+ bool in_streaming;
+

Even though there was no comment previously when this was static, IMO
it is better to comment on all the structure fields where possible.

------
[1] https://www.postgresql.org/message-id/ZQk1Ca_eFDTmBiZy%40paquier.xyz

Kind Regards,
Peter Smith.
Fujitsu Australia