Re: RFC: Logging plan of the running query - Mailing list pgsql-hackers

On Mon, Feb 26, 2024 at 12:19:42PM +0530, Ashutosh Bapat wrote:
> On Sun, Feb 25, 2024 at 5:00 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > > > On Fri, Feb 23, 2024 at 7:50 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > > >
> > > > > But it doesn't have to be all or nothing right?  I mean each call could say
> > > > > what the situation is like in their context, like
> > > > > CHECK_FOR_INTERRUPTS(GUARANTEE_NO_HEAVYWEIGHT_LOCK | GUARANTEE_WHATEVER), and
> > > > > slowly tag calls as needed, similarly to how we add already CFI based on users
> > > > > report.
>
> That has some potential ...
>
> >
> > I might be missing something, but since we already have a ton of macro hacks,
> > why not get another to allow CFI() overloading rather than modifying every
> > single call?  Something like that should do the trick (and CFIFlagHandler() is
> > just a naive example with a function call to avoid multiple evaluation, should
> > be replaced with anything that required more than 10s thinking):
> >
> > #define CHECK_FOR_INTERRUPTS_0() \
> > do { \
> >         if (INTERRUPTS_PENDING_CONDITION()) \
> >                 ProcessInterrupts(); \
> > } while(0)
> >
> > #define CHECK_FOR_INTERRUPTS_1(f) \
> > do { \
> >         if (INTERRUPTS_PENDING_CONDITION()) \
> >                 ProcessInterrupts(); \
> >         \
> >         CFIFlagHandler(f); \
> > } while(0)
>
> From your earlier description I thought you are talking about flags
> that can be ORed. We need only two macros above. Why are we calling
> CFIFLagHandler() after ProcessInterrupts()? Shouldn't we pass flags to
> ProcessInterrupts() itself.

Yes, I'm still talking about ORed flags passed to CFI().  That CFIFlagHandler
call is just an example for a generalized function that would act those flags,
rather than having it coded inside the macro.
>
> >
> > #define CHECK_FOR_INTERRUPTS_X(x, f, CFI_IMPL, ...) CFI_IMPL
> >
> > #define CHECK_FOR_INTERRUPTS(...) \
> >         CHECK_FOR_INTERRUPTS_X(, ##__VA_ARGS__, \
> >                                                         CHECK_FOR_INTERRUPTS_1(__VA_ARGS__), \
> >                                                         CHECK_FOR_INTERRUPTS_0(__VA_ARGS__) \
> >                                                 )
> >
> > We would have to duplicate the current CFI body, but it should never really
> > change, and we shouldn't end up with more than 2 overloads anyway so I don't
> > see it being much of a problem.
>
> Why do we need this complex macro?

So that client code can use either CHECK_FOR_INTERRUPTS() or
CHECK_FOR_INTERRUPTS(flag) rather that transforming every single
CHECK_FOR_INTERRUPTS() to CHECK_FOR_INTERRUPTS(0), which was Robert's
complaint about this approach.