Code review for XML patch. Instill a bit of sanity in the location of

the XmlExpr code in various lists, use a representation that has some hope
of reverse-listing correctly (though it's still a de-escaping function
shy of correctness), generally try to make it look more like Postgres
coding conventions.

3 files changed, 44 insertions, 41 deletions

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 1db920aa0c..f8e8f15a5d 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.163 2006/12/21 16:05:16 petere Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.164 2006/12/24 00:29:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -711,6 +711,18 @@ typedef struct MinMaxExprState
 } MinMaxExprState;
 
 /* ----------------
+ *		XmlExprState node
+ * ----------------
+ */
+typedef struct XmlExprState
+{
+	ExprState	xprstate;
+	List	   *named_args;		/* ExprStates for named arguments */
+	FmgrInfo   *named_outfuncs;	/* array of output fns for named arguments */
+	List	   *args;			/* ExprStates for other arguments */
+} XmlExprState;
+
+/* ----------------
  *		NullTestState node
  * ----------------
  */
@@ -724,22 +736,6 @@ typedef struct NullTestState
 } NullTestState;
 
 /* ----------------
- *		XmlExprState node
- * ----------------
- */
-typedef struct XmlExprState
-{
-	ExprState	xprstate;
-	XmlExprOp	op;
-	char	   *name;
-	List	   *named_args;
-	List	   *args;
-	Oid		   *named_args_tcache;
-	char	  **named_args_ncache;
-	Oid			arg_typeout;
-} XmlExprState;
-
-/* ----------------
  *		CoerceToDomainState node
  * ----------------
  */
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index eb6ba18ada..fa9dff9d96 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/nodes.h,v 1.189 2006/12/21 16:05:16 petere Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/nodes.h,v 1.190 2006/12/24 00:29:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -130,6 +130,7 @@ typedef enum NodeTag
 	T_RowCompareExpr,
 	T_CoalesceExpr,
 	T_MinMaxExpr,
+	T_XmlExpr,
 	T_NullIfExpr,
 	T_NullTest,
 	T_BooleanTest,
@@ -140,7 +141,6 @@ typedef enum NodeTag
 	T_RangeTblRef,
 	T_JoinExpr,
 	T_FromExpr,
-	T_XmlExpr,
 
 	/*
 	 * TAGS FOR EXPRESSION STATE NODES (execnodes.h)
@@ -166,10 +166,10 @@ typedef enum NodeTag
 	T_RowCompareExprState,
 	T_CoalesceExprState,
 	T_MinMaxExprState,
+	T_XmlExprState,
 	T_NullTestState,
 	T_CoerceToDomainState,
 	T_DomainConstraintState,
-	T_XmlExprState,
 
 	/*
 	 * TAGS FOR PLANNER NODES (relation.h)
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 5946300c97..bd2c39040f 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/primnodes.h,v 1.120 2006/12/23 00:43:13 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/primnodes.h,v 1.121 2006/12/24 00:29:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -712,6 +712,33 @@ typedef struct MinMaxExpr
 } MinMaxExpr;
 
 /*
+ * XmlExpr - various SQL/XML functions requiring special grammar productions
+ *
+ * 'name' carries the "NAME foo" argument (already XML-escaped).
+ * 'named_args' and 'arg_names' represent an xml_attribute list.
+ * 'args' carries all other arguments.
+ */
+typedef enum XmlExprOp
+{
+	IS_XMLCONCAT,				/* XMLCONCAT(args) */
+	IS_XMLELEMENT,				/* XMLELEMENT(name, xml_attributes, args) */
+	IS_XMLFOREST,				/* XMLFOREST(xml_attributes) */
+	IS_XMLPARSE,				/* XMLPARSE(text, is_doc, preserve_ws) */
+	IS_XMLPI,					/* XMLPI(name [, args]) */
+	IS_XMLROOT					/* XMLROOT(xml, version, standalone) */
+} XmlExprOp;
+
+typedef struct XmlExpr
+{
+	Expr		xpr;
+	XmlExprOp	op;				/* xml function ID */
+	char	   *name;			/* name in xml(NAME foo ...) syntaxes */
+	List	   *named_args;		/* non-XML expressions for xml_attributes */
+	List	   *arg_names;		/* parallel list of Value strings */
+	List	   *args;			/* list of expressions */
+} XmlExpr;
+
+/*
  * NullIfExpr - a NULLIF expression
  *
  * Like DistinctExpr, this is represented the same as an OpExpr referencing
@@ -766,26 +793,6 @@ typedef struct BooleanTest
 } BooleanTest;
 
 /*
- * XmlExpr - holder for SQL/XML functions XMLCONCAT,
- * XMLELEMENT, XMLFOREST
- */
-typedef enum XmlExprOp
-{
-	IS_XMLCONCAT,
-	IS_XMLELEMENT,
-	IS_XMLFOREST,
-} XmlExprOp;
-
-typedef struct XmlExpr
-{
-	Expr		xpr;
-	XmlExprOp	op;				/* xml expression type */
-	char	   *name;			/* element name */
-	List	   *named_args;
-	List	   *args;
-} XmlExpr;
-
-/*
  * CoerceToDomain
  *
  * CoerceToDomain represents the operation of coercing a value to a domain