Re: Is it useful to record whether plans are generic or custom?

At Wed, 10 Jun 2020 10:50:58 +0900, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> wrote in
> On 2020-06-08 20:45, Masahiro Ikeda wrote:
>
> > BTW, I found that the dependency between function's comments and
> > the modified code is broken at latest patch. Before this is
> > committed, please fix it.
> > ```
> > diff --git a/src/backend/commands/prepare.c
> > b/src/backend/commands/prepare.c
> > index 990782e77f..b63d3214df 100644
> > --- a/src/backend/commands/prepare.c
> > +++ b/src/backend/commands/prepare.c
> > @@ -694,7 +694,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt,
> > IntoClause *into, ExplainState *es,
> > /*
> > * This set returning function reads all the prepared statements and
> > - * returns a set of (name, statement, prepare_time, param_types,
> > - * from_sql).
> > + * returns a set of (name, statement, prepare_time, param_types,
> > from_sql,
> > + * generic_plans, custom_plans, last_plan).
> > */
> > Datum
> > pg_prepared_statement(PG_FUNCTION_ARGS)
> > ```
>
> Thanks for reviewing!
>
> I've fixed it.

+ TupleDescInitEntry(tupdesc, (AttrNumber) 8, "last_plan",

This could be a problem if we showed the last plan in this view. I
think "last_plan_type" would be better.

+ if (prep_stmt->plansource->last_plan_type == PLAN_CACHE_TYPE_CUSTOM)
+ values[7] = CStringGetTextDatum("custom");
+ else if (prep_stmt->plansource->last_plan_type == PLAN_CACHE_TYPE_GENERIC)
+ values[7] = CStringGetTextDatum("generic");
+ else
+ nulls[7] = true;

Using swith-case prevents future additional type (if any) from being
unhandled. I think we are recommending that as a convension.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center