On Mon, Dec 5, 2022 at 8:59 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Sun, Dec 4, 2022 at 5:14 PM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.com> wrote: > > > > On Saturday, December 3, 2022 7:37 PM Amit Kapila <amit.kapil...@gmail.com> > > wrote: > > > Apart from the above, I have slightly adjusted the comments in the > > > attached. Do > > > let me know what you think of the attached. > > > > Thanks for updating the patch. It looks good to me. > > > > I feel the function name ReorderBufferLargestTopTXN() is slightly > misleading because it also checks some of the streaming properties > (like whether the TXN has partial changes and whether it contains any > streamable change). Shall we rename it to > ReorderBufferLargestStreamableTopTXN() or something like that?
Yes that makes sense > The other point to consider is whether we need to have a test case for > this patch. I think before this patch if the size of DDL changes in a > transaction exceeds logical_decoding_work_mem, the empty streams will > be output in the plugin but after this patch, there won't be any such > stream. Yes, we can do that, I will make these two changes. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
