On Mon, Feb 10, 2020 at 1:52 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Feb 7, 2020 at 4:18 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Wed, Feb 5, 2020 at 4:05 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Wed, Feb 5, 2020 at 9:46 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > > > > > Fixed in the latest version sent upthread. > > > > > > > > > > Okay, thanks. I haven't looked at the latest version of patch series > > > as I was reviewing the previous version and I think all of these > > > comments are in the patch which is not modified. Here are my > > > comments: > > > > > > I think we don't need to maintain > > > v8-0007-Support-logical_decoding_work_mem-set-from-create as per > > > discussion in one of the above emails [1] as its usage is not clear. > > > > > > v8-0008-Add-support-for-streaming-to-built-in-replication > > > 1. > > > - information. The allowed options are <literal>slot_name</literal> > > > and > > > - <literal>synchronous_commit</literal> > > > + information. The allowed options are <literal>slot_name</literal>, > > > + <literal>synchronous_commit</literal>, <literal>work_mem</literal> > > > + and <literal>streaming</literal>. > > > > > > As per the discussion above [1], I don't think we need work_mem here. > > > You might want to remove the other usage from the patch as well. > > > > After putting more thought on this it appears that there could be some > > use cases for setting the work_mem from the subscription, Assume a > > case where data are coming from two different origins and based on the > > origin ids different slots might collect different type of changes, > > So isn't it good to have different work_mem for different slots? I am > > not saying that the current way of implementing is the best one but > > that we can improve. First, we need to decide whether we have a use > > case for this or not. > > > > That is the whole point. I don't see a very clear usage of this and > neither did anybody explained clearly how it will be useful. I am not > denying that what you are describing has no use, but as you said we > might need to invent an entirely new way even if we have such a use. > I think it is better to avoid the requirements which are not essential > for this patch.
Ok, I will include this change in the next patch set. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
