From af119e08fd147e808927b1a4c9f091636d52eb45 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 20 Jul 2022 13:54:25 -0400 Subject: [PATCH] Dump more fields when dumping planner internal data structures. Commit 964d01ae9 marked a lot of fields as read_write_ignore to stay consistent with what was dumped by the manually-maintained outfuncs.c code. However, it seems that a pretty fair number of those omissions were either flat-out oversights, or a shortcut taken because hand-written code seemed like it'd be too much trouble. Let's upgrade things where it seems to make sense to dump. To do this, we need to add support to gen_node_support.pl and outfuncs.c for variable-length arrays of Node pointers. That's pretty straightforward given the model of the existing code for arrays of scalars, but I found I needed to tighten the type-recognizing regexes in gen_node_support.pl. (As they stood, they mistook "foo **" for "foo *". Make sure they're all fully anchored to prevent additional problems.) The main thing left un-done here is that a lot of partitioning-related structs are still not dumped, because they are bare structs not Nodes. I'm not sure about the wisdom of that choice ... but changing it would be fairly invasive, so it probably requires more justification than just making planner node dumps more complete. Discussion: https://postgr.es/m/1295668.1658258637@sss.pgh.pa.us --- src/backend/nodes/gen_node_support.pl | 68 ++++++++++++++------- src/backend/nodes/outfuncs.c | 28 +++++++++ src/include/nodes/pathnodes.h | 88 +++++++++++++++------------ 3 files changed, 123 insertions(+), 61 deletions(-) diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl index f3309c3000..86cf1b39d0 100644 --- a/src/backend/nodes/gen_node_support.pl +++ b/src/backend/nodes/gen_node_support.pl @@ -28,8 +28,7 @@ use Catalog; # for RenameTempFile my $output_path = '.'; -GetOptions( - 'outdir:s' => \$output_path) +GetOptions('outdir:s' => \$output_path) or die "$0: wrong arguments"; @@ -441,6 +440,8 @@ foreach my $infile (@ARGV) $type =~ s/\s*$//; # strip space between type and "*" (pointer) */ $type =~ s/\s+\*$/*/; + # strip space between type and "**" (array of pointers) */ + $type =~ s/\s+\*\*$/**/; die "$infile:$lineno: cannot parse data type in \"$line\"\n" @@ -583,7 +584,8 @@ my $header_comment = # nodetags.h push @output_files, 'nodetags.h'; -open my $nt, '>', "$output_path/nodetags.h$tmpext" or die "$output_path/nodetags.h$tmpext: $!"; +open my $nt, '>', "$output_path/nodetags.h$tmpext" + or die "$output_path/nodetags.h$tmpext: $!"; printf $nt $header_comment, 'nodetags.h'; @@ -745,8 +747,8 @@ _equal${n}(const $n *a, const $n *b) unless $equal_ignore || $t eq 'CoercionForm'; } } - # scalar type pointer - elsif ($t =~ /(\w+)\*/ and elem $1, @scalar_types) + # arrays of scalar types + elsif ($t =~ /^(\w+)\*$/ and elem $1, @scalar_types) { my $tt = $1; if (!defined $array_size_field) @@ -780,13 +782,14 @@ _equal${n}(const $n *a, const $n *b) print $eff "\tCOMPARE_SCALAR_FIELD($f);\n" unless $equal_ignore; } # node type - elsif ($t =~ /(\w+)\*/ and elem $1, @node_types) + elsif (($t =~ /^(\w+)\*$/ or $t =~ /^struct\s+(\w+)\*$/) + and elem $1, @node_types) { print $cff "\tCOPY_NODE_FIELD($f);\n" unless $copy_ignore; print $eff "\tCOMPARE_NODE_FIELD($f);\n" unless $equal_ignore; } # array (inline) - elsif ($t =~ /\w+\[/) + elsif ($t =~ /^\w+\[\w+\]$/) { print $cff "\tCOPY_ARRAY_FIELD($f);\n" unless $copy_ignore; print $eff "\tCOMPARE_ARRAY_FIELD($f);\n" unless $equal_ignore; @@ -894,11 +897,16 @@ _read${n}(void) my @a = @{ $node_type_info{$n}->{field_attrs}{$f} }; # extract per-field attributes - my $read_write_ignore = 0; + my $array_size_field; my $read_as_field; + my $read_write_ignore = 0; foreach my $a (@a) { - if ($a =~ /^read_as\(([\w.]+)\)$/) + if ($a =~ /^array_size\(([\w.]+)\)$/) + { + $array_size_field = $1; + } + elsif ($a =~ /^read_as\(([\w.]+)\)$/) { $read_as_field = $1; } @@ -1015,19 +1023,10 @@ _read${n}(void) print $off "\tWRITE_ENUM_FIELD($f, $t);\n"; print $rff "\tREAD_ENUM_FIELD($f, $t);\n" unless $no_read; } - # arrays - elsif ($t =~ /(\w+)(\*|\[)/ and elem $1, @scalar_types) + # arrays of scalar types + elsif ($t =~ /^(\w+)(\*|\[\w+\])$/ and elem $1, @scalar_types) { my $tt = uc $1; - my $array_size_field; - foreach my $a (@a) - { - if ($a =~ /^array_size\(([\w.]+)\)$/) - { - $array_size_field = $1; - last; - } - } if (!defined $array_size_field) { die "no array size defined for $n.$f of type $t\n"; @@ -1080,11 +1079,38 @@ _read${n}(void) . "\t\toutBitmapset(str, NULL);\n"; } # node type - elsif ($t =~ /(\w+)\*/ and elem $1, @node_types) + elsif (($t =~ /^(\w+)\*$/ or $t =~ /^struct\s+(\w+)\*$/) + and elem $1, @node_types) { print $off "\tWRITE_NODE_FIELD($f);\n"; print $rff "\tREAD_NODE_FIELD($f);\n" unless $no_read; } + # arrays of node pointers (currently supported for write only) + elsif (($t =~ /^(\w+)\*\*$/ or $t =~ /^struct\s+(\w+)\*\*$/) + and elem($1, @node_types)) + { + if (!defined $array_size_field) + { + die "no array size defined for $n.$f of type $t\n"; + } + if ($node_type_info{$n}->{field_types}{$array_size_field} eq + 'List*') + { + print $off + "\tWRITE_NODE_ARRAY($f, list_length(node->$array_size_field));\n"; + print $rff + "\tREAD_NODE_ARRAY($f, list_length(local_node->$array_size_field));\n" + unless $no_read; + } + else + { + print $off + "\tWRITE_NODE_ARRAY($f, node->$array_size_field);\n"; + print $rff + "\tREAD_NODE_ARRAY($f, local_node->$array_size_field);\n" + unless $no_read; + } + } elsif ($t eq 'struct CustomPathMethods*' || $t eq 'struct CustomScanMethods*') { diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 4077fc4498..a96f2ee8c6 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -97,6 +97,11 @@ static void outChar(StringInfo str, char c); (appendStringInfoString(str, " :" CppAsString(fldname) " "), \ outBitmapset(str, node->fldname)) +/* Write a variable-length array (not a List) of Node pointers */ +#define WRITE_NODE_ARRAY(fldname, len) \ + (appendStringInfoString(str, " :" CppAsString(fldname) " "), \ + writeNodeArray(str, (const Node * const *) node->fldname, len)) + /* Write a variable-length array of AttrNumber */ #define WRITE_ATTRNUMBER_ARRAY(fldname, len) \ (appendStringInfoString(str, " :" CppAsString(fldname) " "), \ @@ -208,6 +213,29 @@ WRITE_SCALAR_ARRAY(writeIndexCols, Index, " %u",) WRITE_SCALAR_ARRAY(writeIntCols, int, " %d",) WRITE_SCALAR_ARRAY(writeBoolCols, bool, " %s", booltostr) +/* + * Print an array (not a List) of Node pointers. + * + * The decoration is identical to that of scalar arrays, but we can't + * quite use appendStringInfo() in the loop. + */ +static void +writeNodeArray(StringInfo str, const Node *const *arr, int len) +{ + if (arr != NULL) + { + appendStringInfoChar(str, '('); + for (int i = 0; i < len; i++) + { + appendStringInfoChar(str, ' '); + outNode(str, arr[i]); + } + appendStringInfoChar(str, ')'); + } + else + appendStringInfoString(str, "<>"); +} + /* * Print a List. */ diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index e650af5ff2..e2081db4ed 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -89,7 +89,7 @@ typedef enum UpperRelationKind * planned. * * Not all fields are printed. (In some cases, there is no print support for - * the field type.) + * the field type; in others, doing so would lead to infinite recursion.) *---------- */ typedef struct PlannerGlobal @@ -177,7 +177,8 @@ typedef struct PlannerGlobal * either here or in that header, whichever is read first. * * Not all fields are printed. (In some cases, there is no print support for - * the field type.) + * the field type; in others, doing so would lead to infinite recursion or + * bloat dump output more than seems useful.) *---------- */ #ifndef HAVE_PLANNERINFO_TYPEDEF @@ -220,14 +221,15 @@ struct PlannerInfo * does not correspond to a base relation, such as a join RTE or an * unreferenced view RTE; or if the RelOptInfo hasn't been made yet. */ - struct RelOptInfo **simple_rel_array pg_node_attr(read_write_ignore); + struct RelOptInfo **simple_rel_array pg_node_attr(array_size(simple_rel_array_size)); /* allocated size of array */ - int simple_rel_array_size pg_node_attr(read_write_ignore); + int simple_rel_array_size; /* * simple_rte_array is the same length as simple_rel_array and holds * pointers to the associated rangetable entries. Using this is a shade - * faster than using rt_fetch(), mostly due to fewer indirections. + * faster than using rt_fetch(), mostly due to fewer indirections. (Not + * printed because it'd be redundant with parse->rtable.) */ RangeTblEntry **simple_rte_array pg_node_attr(read_write_ignore); @@ -235,7 +237,8 @@ struct PlannerInfo * append_rel_array is the same length as the above arrays, and holds * pointers to the corresponding AppendRelInfo entry indexed by * child_relid, or NULL if the rel is not an appendrel child. The array - * itself is not allocated if append_rel_list is empty. + * itself is not allocated if append_rel_list is empty. (Not printed + * because it'd be redundant with append_rel_list.) */ struct AppendRelInfo **append_rel_array pg_node_attr(read_write_ignore); @@ -273,6 +276,9 @@ struct PlannerInfo * join_cur_level is the current level. New join-relation RelOptInfos are * automatically added to the join_rel_level[join_cur_level] list. * join_rel_level is NULL if not in use. + * + * Note: we've already printed all baserel and joinrel RelOptInfos above, + * so we don't dump join_rel_level or other lists of RelOptInfos. */ /* lists of join-relation RelOptInfos */ List **join_rel_level pg_node_attr(read_write_ignore); @@ -403,8 +409,8 @@ struct PlannerInfo /* * Fields filled during create_plan() for use in setrefs.c */ - /* for GroupingFunc fixup */ - AttrNumber *grouping_map pg_node_attr(array_size(update_colnos), read_write_ignore); + /* for GroupingFunc fixup (can't print: array length not known here) */ + AttrNumber *grouping_map pg_node_attr(read_write_ignore); /* List of MinMaxAggInfos */ List *minmax_aggs; @@ -458,7 +464,7 @@ struct PlannerInfo /* PARAM_EXEC ID for the work table */ int wt_param_id; /* a path for non-recursive term */ - struct Path *non_recursive_path pg_node_attr(read_write_ignore); + struct Path *non_recursive_path; /* * These fields are workspace for createplan.c @@ -470,7 +476,9 @@ struct PlannerInfo /* * These fields are workspace for setrefs.c. Each is an array - * corresponding to glob->subplans. + * corresponding to glob->subplans. (We could probably teach + * gen_node_support.pl how to determine the array length, but it doesn't + * seem worth the trouble, so just mark them read_write_ignore.) */ bool *isAltSubplan pg_node_attr(read_write_ignore); bool *isUsedSubplan pg_node_attr(read_write_ignore); @@ -928,16 +936,17 @@ typedef struct RelOptInfo * Number of partitions; -1 if not yet set; in case of a join relation 0 * means it's considered unpartitioned */ - int nparts pg_node_attr(read_write_ignore); + int nparts; /* Partition bounds */ struct PartitionBoundInfoData *boundinfo pg_node_attr(read_write_ignore); /* True if partition bounds were created by partition_bounds_merge() */ bool partbounds_merged; /* Partition constraint, if not the root */ - List *partition_qual pg_node_attr(read_write_ignore); + List *partition_qual; /* * Array of RelOptInfos of partitions, stored in the same order as bounds + * (don't print, too bulky and duplicative) */ struct RelOptInfo **part_rels pg_node_attr(read_write_ignore); @@ -948,6 +957,12 @@ typedef struct RelOptInfo Bitmapset *live_parts; /* Relids set of all partition relids */ Relids all_partrels; + + /* + * These arrays are of length partkey->partnatts, which we don't have at + * hand, so don't try to print + */ + /* Non-nullable partition key expressions */ List **partexprs pg_node_attr(read_write_ignore); /* Nullable partition key expressions */ @@ -1042,30 +1057,26 @@ struct IndexOptInfo int nkeycolumns; /* - * array fields aren't really worth the trouble to print + * table column numbers of index's columns (both key and included + * columns), or 0 for expression columns */ - - /* - * column numbers of index's attributes both key and included columns, or - * 0 - */ - int *indexkeys pg_node_attr(read_write_ignore); + int *indexkeys pg_node_attr(array_size(ncolumns)); /* OIDs of collations of index columns */ - Oid *indexcollations pg_node_attr(read_write_ignore); + Oid *indexcollations pg_node_attr(array_size(nkeycolumns)); /* OIDs of operator families for columns */ - Oid *opfamily pg_node_attr(read_write_ignore); + Oid *opfamily pg_node_attr(array_size(nkeycolumns)); /* OIDs of opclass declared input data types */ - Oid *opcintype pg_node_attr(read_write_ignore); + Oid *opcintype pg_node_attr(array_size(nkeycolumns)); /* OIDs of btree opfamilies, if orderable */ - Oid *sortopfamily pg_node_attr(read_write_ignore); + Oid *sortopfamily pg_node_attr(array_size(nkeycolumns)); /* is sort order descending? */ - bool *reverse_sort pg_node_attr(read_write_ignore); + bool *reverse_sort pg_node_attr(array_size(nkeycolumns)); /* do NULLs come first in the sort order? */ - bool *nulls_first pg_node_attr(read_write_ignore); + bool *nulls_first pg_node_attr(array_size(nkeycolumns)); /* opclass-specific options for columns */ bytea **opclassoptions pg_node_attr(read_write_ignore); /* which index cols can be returned in an index-only scan? */ - bool *canreturn pg_node_attr(read_write_ignore); + bool *canreturn pg_node_attr(array_size(ncolumns)); /* OID of the access method (in pg_am) */ Oid relam; @@ -1098,19 +1109,19 @@ struct IndexOptInfo /* * Remaining fields are copied from the index AM's API struct - * (IndexAmRoutine). We don't bother to dump them. + * (IndexAmRoutine). */ - bool amcanorderbyop pg_node_attr(read_write_ignore); - bool amoptionalkey pg_node_attr(read_write_ignore); - bool amsearcharray pg_node_attr(read_write_ignore); - bool amsearchnulls pg_node_attr(read_write_ignore); + bool amcanorderbyop; + bool amoptionalkey; + bool amsearcharray; + bool amsearchnulls; /* does AM have amgettuple interface? */ - bool amhasgettuple pg_node_attr(read_write_ignore); + bool amhasgettuple; /* does AM have amgetbitmap interface? */ - bool amhasgetbitmap pg_node_attr(read_write_ignore); - bool amcanparallel pg_node_attr(read_write_ignore); + bool amhasgetbitmap; + bool amcanparallel; /* does AM have ammarkpos interface? */ - bool amcanmarkpos pg_node_attr(read_write_ignore); + bool amcanmarkpos; /* AM's cost estimator */ /* Rather than include amapi.h here, we declare amcostestimate like this */ void (*amcostestimate) () pg_node_attr(read_write_ignore); @@ -1184,12 +1195,9 @@ typedef struct StatisticExtInfo Oid statOid; /* includes child relations */ - bool inherit pg_node_attr(read_write_ignore); + bool inherit; - /* - * back-link to statistic's table; don't print, infinite recursion on plan - * tree dump - */ + /* back-link to statistic's table; don't print, else infinite recursion */ RelOptInfo *rel pg_node_attr(read_write_ignore); /* statistics kind of this entry */