On Mon, Dec 5, 2022 at 8:59 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sun, Dec 4, 2022 at 5:14 PM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On Saturday, December 3, 2022 7:37 PM Amit Kapila <amit.kapila16@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
