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

At Wed, 10 Jun 2020 10:50:58 +0900, torikoshia <torikoshia@oss.nttdata.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