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


Reply via email to