Hi!
> 27 дек. 2018 г., в 12:54, Evgeniy Efimkin <[email protected]> написал(а):
>
> 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 better to write "All tables listed
in clause must present in the publication". But I'm not a native speaker, just
looks that it'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 of following
>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 it a 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.
