Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)

Hi!

> 27 дек. 2018 г., в 12:54, Evgeniy Efimkin <efimkin(at)yandex-team(dot)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 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.