Re: [HACKERS] Inadequate infrastructure for NextValueExpr

On Fri, Jul 14, 2017 at 9:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Somebody decided they could add a new primnode type without bothering to
> build out very much infrastructure for it.  Thus:
>
> regression=# create table foo (f1 int, f2 int generated always as identity);
> CREATE TABLE
> regression=# insert into foo values(1);
> INSERT 0 1
> regression=# explain verbose insert into foo values(1);
> ERROR:  unrecognized node type: 146
>
> because ruleutils.c has never heard of NextValueExpr.  The lack of
> outfuncs/readfuncs support for it is rather distressing as well.
> That doesn't break parallel queries today, because (I think)
> you can't get one of these nodes in a parallelizable query, but it
> is going to cause problems for debugging.  It will also break
> (more or less) pg_stat_statements.  I also wonder whether costsize.c
> oughtn't be charging some estimated execution cost for it.

I wrote a script to cross-check various node handling functions and it told me:

T_NextValueExpr not handled in outfuncs.c
T_ObjectWithArgs not handled in outfuncs.c
T_AccessPriv not handled in outfuncs.c
T_CreateOpClassItem not handled in outfuncs.c
T_FunctionParameter not handled in outfuncs.c
T_InferClause not handled in outfuncs.c
T_OnConflictClause not handled in outfuncs.c
T_RoleSpec not handled in outfuncs.c
T_PartitionCmd not handled in outfuncs.c
T_NamedTuplestoreScan can be produced by outfuncs.c with tagname
NAMEDTUPLESTORESCAN but that tagname is not recognized by readfuncs.c
T_Alias not handled in ruleutils.c
T_RangeVar not handled in ruleutils.c
T_Expr not handled in ruleutils.c
T_CaseWhen not handled in ruleutils.c
T_TargetEntry not handled in ruleutils.c
T_RangeTblRef not handled in ruleutils.c
T_JoinExpr not handled in ruleutils.c
T_FromExpr not handled in ruleutils.c
T_OnConflictExpr not handled in ruleutils.c
T_IntoClause not handled in ruleutils.c
T_NextValueExpr not handled in ruleutils.c

It classifies all node types into categories PLAN NODES, PRIMITIVE
NODES, ... using the per-group comments in nodes.h, and then checks
some rules that I inferred (obviously incorrectly) about which of
those categories should be handled by copy, out, equal, read and
get_rule_expr functions and also checks that readfuncs.c and
outfuncs.c agree on the name string.  It needs some work though:
anyone got any ideas on how to improve the categories and rules to
make it right?  To use this approach, it would need to be the case
that each checked function exhaustively handles a subset of node tags
that can be described by listing those categories.

That revealed a defect in commit
18ce3a4ab22d2984f8540ab480979c851dae5338 which I think should be
corrected with something like the attached, though I'm not sure if
it's possible to reach it.  It would be nice to do something more
mechanised for this type of code...

-- 
Thomas Munro
http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers