Hi!
> 27 дек. 2018 г., в 12:54, Evgeniy Efimkin <efimkin@yandex-team.ru> написал(а):
>
> In latest patch i removed `FOR ALL TABLES` clause and `alltables` parameter, now it's look more simple.
> Add new system view pg_user_subscirption to allow non-superuser use pg_dump and select addition column from
pg_subscrption
> Changes docs.
I've reviewed patch again, here are my notes:
1. In create_subscription.sgml and some others. "All tables listed in clause must be present in publication" I think is
betterto write "All tables listed in clause must present in the publication". But I'm not a native speaker, just looks
thatit'd be good if someone proofread docs..
2. New view should be called pg_user_subscription or pg_user_subscriptions? Nearby views are plural e.g.
pg_publication_tables.
3. I do not know how will this view get into the db during pg_upgrade. Is it somehow handled?
4. In subscriptioncmds.c :
>if (stmt->tables&&!connect)
some spaces needed to make AND readable
5. Same file in CreateSubscription() there's foreach collecting tablesiods. This oids are necessary only in on branch
offollowing
>if (stmt->tables)
May be we should refactor this, move tablesiods closer to the place where they are used?
6. In AlterSubscription_set_table() and AlterSubscription_drop_table() palloced memory is not free()'d. Is it OK or is
ita leak?
7.
> errmsg("table \"%s.%s\" not in preset subscription"
Should it be "table does not present in subscription"?
Besides this, patch looks good to me.
Thanks for working on this!
Best regards, Andrey Borodin.