On Wednesday, November 10, 2021 6:13 PM I wrote: > On Wednesday, November 10, 2021 3:43 PM Dilip Kumar > <dilipbal...@gmail.com> wrote: > > On Tue, Nov 9, 2021 at 5:05 PM osumi.takami...@fujitsu.com > > <osumi.takami...@fujitsu.com> wrote: > > > On Tuesday, November 9, 2021 12:08 PM Greg Nancarrow > > <gregn4...@gmail.com> wrote: > > > > On Fri, Nov 5, 2021 at 7:11 PM osumi.takami...@fujitsu.com > > > > <osumi.takami...@fujitsu.com> wrote: > > > > > > > > > > > > > I did a quick scan through the latest v8 patch and noticed the > > > > following > > things: > > > I appreciate your review ! > > I have reviewed some part of the patch and I have a few comments > I really appreciate your attention and review. ... > > 3. > > + { > > + size += *extra_data->stream_write_len; > > + add_apply_error_context_xact_size(size); > > + return; > > + } > > > > From apply_handle_insert(), we are calling update_apply_change_size(), > > and inside this function we are dereferencing > *extra_data->stream_write_len. > > Basically, stream_write_len is in integer pointer and the caller > > hasn't allocated memory for that and inside update_apply_change_size, > > we are directly dereferencing the pointer, how this can be correct. ... > I'll just delete the top part that handles streaming bytes calculation in the > update_apply_change_size(). > It's because now that there is a specific structure to recognize each > streaming > xid and save transaction size there, which makes the top part in question > useless. > > > And I also see that in the > > whole patch stream_write_len, is never used as lvalue so without > > storing anything into this why are we trying to use this as rvalue > > here? This is clearly an issue. > As described above, I'll fix this part and related codes mainly streaming > related > codes in the next version. Removed this deadcodes.
Please have a look at [1] [1] - https://www.postgresql.org/message-id/TYCPR01MB8373FEB287F733C81C1E4D42ED989%40TYCPR01MB8373.jpnprd01.prod.outlook.com Best Regards, Takamichi Osumi