Re: RFC: Logging plan of the running query

On Sun, Feb 25, 2024 at 5:00 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> On Sat, Feb 24, 2024 at 08:56:41AM -0500, James Coleman wrote:
> > On Fri, Feb 23, 2024 at 10:23 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Feb 23, 2024 at 7:50 PM Julien Rouhaud <rjuju123(at)gmail(dot)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 ...

>
> Yeah, trying to find a generalized solution seems like worth investing some
> time to avoid having a bunch of CHECK_FOR_XXX() calls scattered in the code a
> few years down the road.
>
> 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.

>
> #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?

--
Best Wishes,
Ashutosh Bapat