Hi!
> 29 янв. 2019 г., в 14:51, Evgeniy Efimkin <efim...@yandex-team.ru> написал(а):
>
> <subscription_v2.patch>
Thanks for the next version.
1. In tests the code for "sub reset_pg_hba" is taken from authentication tests
(which is fine), but comments are omitted. Let's take them too?
2. 011_rep_changes_nonsuperuser.pl seems a lot like 001_rep_changes.pl with
auth adjustments
I think it is OK, but if you know some clever way to refactor that it would be
cool. We could avoid duplication of 200 code line of tests or so.
3. I'm not sure, but I'd add some articles here: "All tables listed in _a_
clause must be present in _the_ publication."
"clauses will remove ? table from ? subscription"
4.
>qsort(subrel_local_oids, list_length(subrel_states),
sizeof(Oid), oid_cmp);
is indented two times differently, but I think this will be fixed by pg_indent.
There are some other indentation issues.
/* Load the library providing us libpq calls. */
/* Check the connection info string. */
load_file("libpqwalreceiver", false);
walrcv_check_conninfo(stmt->conninfo);
Here 2nd comment jumped a line up. And there are superfluous new line in that
code block.
Random newline added after
>errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled
>subscriptions")));
5. In regression test we only subtract test (fail for nonsuperuser). Can we add
some test there too?
All these are merely cosmetical issues. I believe after addressing these we can
switch patch to Ready For Committer.
Best regards, Andrey Borodin.
