On Sat, Oct 4, 2025 at 6:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Fri, Oct 3, 2025 at 4:00 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Fri, Oct 3, 2025 at 7:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > I've updated the patch based on your comments. Please find the attached patch.
> >
> > Thanks for updating the patch!
> >
> > +step "s1_tuplock" {
> > + EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
> > + SELECT a.i,
> > + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
> > + FROM a as a
> > + FOR UPDATE;
> > +}
> >
> > Maybe my comment about this step was not enough, but I'm wondering we
> > should run EXPLAIN and then SELECT here like below, rather than
> > running EXPLAIN ANALYZE, as that seems more usual to me:
> >
> > step "s1_tuplock" {
> > EXPLAIN (VERBOSE, COSTS OFF)
> > SELECT a.i,
> > (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
> > FROM a
> > FOR UPDATE;
> > SELECT a.i,
> > (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
> > FROM a
> > FOR UPDATE;
> > }
> >
> > I added the VERBOSE option to show the remote query, and removed the
> > alias for a.
>
> Done.
>
> > BTW: you added quotation marks around a name for each session or step
> > like "s1_tuplock". Do we really need them? This is nitpicking,
> > though.
>
> No, I brought a convention from other spec files. I've unquoted all
> sessions and steps.
>
> I've attached the updated patch that dealt with all comments including
> ones Michael-san made[1].
Thanks for updating the patch!
The isolation test fails. I think you failed to update the expected
file. Also, you left the useless alias for table a alone...
Best regards,
Etsuro Fujita