From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Extending outfuncs support to utility statements
In-reply-to: <1078368.1657548997@sss.pgh.pa.us>
References: <4159834.1657405226@sss.pgh.pa.us>
 <9b197806-17ee-acba-0e70-01263aa46e79@enterprisedb.com>
 <1078368.1657548997@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 11 Jul 2022 10:16:37 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1919613.1657663274.0@sss.pgh.pa.us>
Date: Tue, 12 Jul 2022 18:01:39 -0400
Message-ID: <1919681.1657663299@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1919681.1657663299%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1919613.1657663274.1@sss.pgh.pa.us>

I wrote:
> There might be enough node types that are raw-parse-tree-only,
> but not involved in utility statements, to make it worth
> continuing to suppress readfuncs support for them.  But I kinda
> doubt it.  I'll try to get some numbers later today.

Granting that we want write/read support for utility statements,
it seems that what we can save by suppressing raw-parse-tree-only
nodes is only about 10kB.  That's clearly not worth troubling over
in the grand scheme of things, so I suggest that we just open the
floodgates as attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="remove-remaining-outfunc-readfunc-suppression-hacks.patch";
	charset="us-ascii"
Content-ID: <1919613.1657663274.2@sss.pgh.pa.us>
Content-Description: remove-remaining-outfunc-readfunc-suppression-hacks.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen=
_node_support.pl
index 7694e04d0a..96af17516a 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -166,23 +166,6 @@ push @scalar_types, qw(EquivalenceClass* EquivalenceM=
ember*);
 # currently not required.
 push @scalar_types, qw(QualCost);
 =

-# XXX various things we are not publishing right now to stay level
-# with the manual system
-push @no_read_write,
-  qw(AccessPriv AlterTableCmd CreateOpClassItem FunctionParameter InferCl=
ause ObjectWithArgs OnConflictClause PartitionCmd RoleSpec VacuumRelation)=
;
-push @no_read, qw(A_ArrayExpr A_Indices A_Indirection AlterStatsStmt
-  CollateClause ColumnDef ColumnRef CreateForeignTableStmt CreateStatsStm=
t
-  CreateStmt FuncCall ImportForeignSchemaStmt IndexElem IndexStmt
-  JsonAggConstructor JsonArgument JsonArrayAgg JsonArrayConstructor
-  JsonArrayQueryConstructor JsonCommon JsonFuncExpr JsonKeyValue
-  JsonObjectAgg JsonObjectConstructor JsonOutput JsonParseExpr JsonScalar=
Expr
-  JsonSerializeExpr JsonTable JsonTableColumn JsonTablePlan LockingClause
-  MultiAssignRef PLAssignStmt ParamRef PartitionElem PartitionSpec
-  PlaceHolderVar PublicationObjSpec PublicationTable RangeFunction
-  RangeSubselect RangeTableFunc RangeTableFuncCol RangeTableSample RawStm=
t
-  ResTarget ReturnStmt SelectStmt SortBy StatsElem TableLikeClause
-  TriggerTransition TypeCast TypeName WindowDef WithClause XmlSerialize);
-
 =

 ## check that we have the expected number of files on the command line
 die "wrong number of input files, expected @all_input_files\n"
@@ -795,14 +778,6 @@ foreach my $n (@node_types)
 	next if elem $n, @nodetag_only;
 	next if elem $n, @no_read_write;
 =

-	# XXX For now, skip all "Stmt"s except that ones that were there before.
-	if ($n =3D~ /Stmt$/)
-	{
-		my @keep =3D
-		  qw(AlterStatsStmt CreateForeignTableStmt CreateStatsStmt CreateStmt D=
eclareCursorStmt ImportForeignSchemaStmt IndexStmt NotifyStmt PlannedStmt =
PLAssignStmt RawStmt ReturnStmt SelectStmt SetOperationStmt);
-		next unless elem $n, @keep;
-	}
-
 	my $no_read =3D (elem $n, @no_read);
 =

 	# output format starts with upper case node type name
@@ -827,13 +802,20 @@ _out${n}(StringInfo str, const $n *node)
 =

 ";
 =

-	print $rff "
+	if (!$no_read)
+	{
+		my $macro =3D
+		  (@{ $node_type_info{$n}->{fields} } > 0)
+		  ? 'READ_LOCALS'
+		  : 'READ_LOCALS_NO_FIELDS';
+		print $rff "
 static $n *
 _read${n}(void)
 {
-\tREAD_LOCALS($n);
+\t$macro($n);
 =

-" unless $no_read;
+";
+	}
 =

 	# print instructions for each field
 	foreach my $f (@{ $node_type_info{$n}->{fields} })
@@ -883,6 +865,7 @@ _read${n}(void)
 			print $rff "\tREAD_LOCATION_FIELD($f);\n" unless $no_read;
 		}
 		elsif ($t eq 'int'
+			|| $t eq 'int16'
 			|| $t eq 'int32'
 			|| $t eq 'AttrNumber'
 			|| $t eq 'StrategyNumber')
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 4d776e7b51..85ac0b5e21 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -415,79 +415,6 @@ _outExtensibleNode(StringInfo str, const ExtensibleNo=
de *node)
 	methods->nodeOut(str, node);
 }
 =

-static void
-_outQuery(StringInfo str, const Query *node)
-{
-	WRITE_NODE_TYPE("QUERY");
-
-	WRITE_ENUM_FIELD(commandType, CmdType);
-	WRITE_ENUM_FIELD(querySource, QuerySource);
-	/* we intentionally do not print the queryId field */
-	WRITE_BOOL_FIELD(canSetTag);
-
-	/*
-	 * Hack to work around missing outfuncs routines for a lot of the
-	 * utility-statement node types.  (The only one we actually *need* for
-	 * rules support is NotifyStmt.)  Someday we ought to support 'em all, b=
ut
-	 * for the meantime do this to avoid getting lots of warnings when runni=
ng
-	 * with debug_print_parse on.
-	 */
-	if (node->utilityStmt)
-	{
-		switch (nodeTag(node->utilityStmt))
-		{
-			case T_CreateStmt:
-			case T_IndexStmt:
-			case T_NotifyStmt:
-			case T_DeclareCursorStmt:
-				WRITE_NODE_FIELD(utilityStmt);
-				break;
-			default:
-				appendStringInfoString(str, " :utilityStmt ?");
-				break;
-		}
-	}
-	else
-		appendStringInfoString(str, " :utilityStmt <>");
-
-	WRITE_INT_FIELD(resultRelation);
-	WRITE_BOOL_FIELD(hasAggs);
-	WRITE_BOOL_FIELD(hasWindowFuncs);
-	WRITE_BOOL_FIELD(hasTargetSRFs);
-	WRITE_BOOL_FIELD(hasSubLinks);
-	WRITE_BOOL_FIELD(hasDistinctOn);
-	WRITE_BOOL_FIELD(hasRecursive);
-	WRITE_BOOL_FIELD(hasModifyingCTE);
-	WRITE_BOOL_FIELD(hasForUpdate);
-	WRITE_BOOL_FIELD(hasRowSecurity);
-	WRITE_BOOL_FIELD(isReturn);
-	WRITE_NODE_FIELD(cteList);
-	WRITE_NODE_FIELD(rtable);
-	WRITE_NODE_FIELD(jointree);
-	WRITE_NODE_FIELD(targetList);
-	WRITE_ENUM_FIELD(override, OverridingKind);
-	WRITE_NODE_FIELD(onConflict);
-	WRITE_NODE_FIELD(returningList);
-	WRITE_NODE_FIELD(groupClause);
-	WRITE_BOOL_FIELD(groupDistinct);
-	WRITE_NODE_FIELD(groupingSets);
-	WRITE_NODE_FIELD(havingQual);
-	WRITE_NODE_FIELD(windowClause);
-	WRITE_NODE_FIELD(distinctClause);
-	WRITE_NODE_FIELD(sortClause);
-	WRITE_NODE_FIELD(limitOffset);
-	WRITE_NODE_FIELD(limitCount);
-	WRITE_ENUM_FIELD(limitOption, LimitOption);
-	WRITE_NODE_FIELD(rowMarks);
-	WRITE_NODE_FIELD(setOperations);
-	WRITE_NODE_FIELD(constraintDeps);
-	WRITE_NODE_FIELD(withCheckOptions);
-	WRITE_NODE_FIELD(mergeActionList);
-	WRITE_BOOL_FIELD(mergeUseOuterJoin);
-	WRITE_LOCATION_FIELD(stmt_location);
-	WRITE_INT_FIELD(stmt_len);
-}
-
 static void
 _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
 {
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 1421686938..a2391280be 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -240,56 +240,6 @@ readBitmapset(void)
  * special_read_write attribute
  */
 =

-static Query *
-_readQuery(void)
-{
-	READ_LOCALS(Query);
-
-	READ_ENUM_FIELD(commandType, CmdType);
-	READ_ENUM_FIELD(querySource, QuerySource);
-	local_node->queryId =3D UINT64CONST(0);	/* not saved in output format */
-	READ_BOOL_FIELD(canSetTag);
-	READ_NODE_FIELD(utilityStmt);
-	READ_INT_FIELD(resultRelation);
-	READ_BOOL_FIELD(hasAggs);
-	READ_BOOL_FIELD(hasWindowFuncs);
-	READ_BOOL_FIELD(hasTargetSRFs);
-	READ_BOOL_FIELD(hasSubLinks);
-	READ_BOOL_FIELD(hasDistinctOn);
-	READ_BOOL_FIELD(hasRecursive);
-	READ_BOOL_FIELD(hasModifyingCTE);
-	READ_BOOL_FIELD(hasForUpdate);
-	READ_BOOL_FIELD(hasRowSecurity);
-	READ_BOOL_FIELD(isReturn);
-	READ_NODE_FIELD(cteList);
-	READ_NODE_FIELD(rtable);
-	READ_NODE_FIELD(jointree);
-	READ_NODE_FIELD(targetList);
-	READ_ENUM_FIELD(override, OverridingKind);
-	READ_NODE_FIELD(onConflict);
-	READ_NODE_FIELD(returningList);
-	READ_NODE_FIELD(groupClause);
-	READ_BOOL_FIELD(groupDistinct);
-	READ_NODE_FIELD(groupingSets);
-	READ_NODE_FIELD(havingQual);
-	READ_NODE_FIELD(windowClause);
-	READ_NODE_FIELD(distinctClause);
-	READ_NODE_FIELD(sortClause);
-	READ_NODE_FIELD(limitOffset);
-	READ_NODE_FIELD(limitCount);
-	READ_ENUM_FIELD(limitOption, LimitOption);
-	READ_NODE_FIELD(rowMarks);
-	READ_NODE_FIELD(setOperations);
-	READ_NODE_FIELD(constraintDeps);
-	READ_NODE_FIELD(withCheckOptions);
-	READ_NODE_FIELD(mergeActionList);
-	READ_BOOL_FIELD(mergeUseOuterJoin);
-	READ_LOCATION_FIELD(stmt_location);
-	READ_INT_FIELD(stmt_len);
-
-	READ_DONE();
-}
-
 static Const *
 _readConst(void)
 {
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes=
.h
index e2ad761768..29e48642b7 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -117,8 +117,6 @@ typedef uint32 AclMode;			/* a bitmask of privilege bi=
ts */
  */
 typedef struct Query
 {
-	pg_node_attr(custom_read_write)
-
 	NodeTag		type;
 =

 	CmdType		commandType;	/* select|insert|update|delete|merge|utility */
@@ -126,10 +124,10 @@ typedef struct Query
 	QuerySource querySource;	/* where did I come from? */
 =

 	/*
-	 * query identifier (can be set by plugins); ignored for equal, might no=
t
-	 * be set
+	 * query identifier (can be set by plugins); ignored for equal, as it
+	 * might not be set; also not stored
 	 */
-	uint64		queryId pg_node_attr(equal_ignore, read_as(0));
+	uint64		queryId pg_node_attr(equal_ignore, read_write_ignore, read_as(0)=
);
 =

 	bool		canSetTag;		/* do I set the command result tag? */
 =


------- =_aaaaaaaaaa0--