Hi!
> 29 янв. 2019 г., в 14:51, Evgeniy Efimkin <efimkin@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
codeline 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.