|From:||Andrey Borodin <x4mmm(at)yandex-team(dot)ru>|
|To:||Evgeniy Efimkin <efimkin(at)yandex-team(dot)ru>|
|Cc:||Stephen Frost <sfrost(at)snowman(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Дмитрий Сарафанников <dsarafan(at)yandex-team(dot)ru>, Владимир Бородин <root(at)simply(dot)name>|
|Subject:||Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)|
|Views:||Raw Message | Whole Thread | Download mbox|
> 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 :
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
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?
> 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.
|Next Message||Dean Rasheed||2019-01-14 09:09:39||Re: [HACKERS] PATCH: multivariate histograms and MCV lists|
|Previous Message||Andres Freund||2019-01-14 06:39:42||Re: Reducing header interdependencies around heapam.h et al.|