xmlconcat as variadic function

Here is a patch to reimplement the xmlconcat functionality as a variadic
function instead of a hardcoded special expression type.  I haven't
found any variadic function in the set of built-in functions so far, so
I figured I would ask for feedback before we go down this route.

Btw., the corresponding ecpg changes appear to have fallen into place
automatically.  Nice.
Index: src/backend/executor/execQual.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execQual.c,v
retrieving revision 1.237
diff -u -3 -p -c -r1.237 execQual.c
*** src/backend/executor/execQual.c    15 Nov 2008 20:52:35 -0000    1.237
--- src/backend/executor/execQual.c    17 Nov 2008 11:17:09 -0000
*************** ExecEvalXml(XmlExprState *xmlExpr, ExprC
*** 3174,3202 ****

      switch (xexpr->op)
      {
-         case IS_XMLCONCAT:
-             {
-                 List       *values = NIL;
-
-                 foreach(arg, xmlExpr->args)
-                 {
-                     ExprState  *e = (ExprState *) lfirst(arg);
-
-                     value = ExecEvalExpr(e, econtext, &isnull, NULL);
-                     if (!isnull)
-                         values = lappend(values, DatumGetPointer(value));
-                 }
-
-                 if (list_length(values) > 0)
-                 {
-                     *isNull = false;
-                     return PointerGetDatum(xmlconcat(values));
-                 }
-                 else
-                     return (Datum) 0;
-             }
-             break;
-
          case IS_XMLFOREST:
          {
              StringInfoData buf;
--- 3174,3179 ----
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.637
diff -u -3 -p -c -r2.637 gram.y
*** src/backend/parser/gram.y    13 Nov 2008 11:10:06 -0000    2.637
--- src/backend/parser/gram.y    17 Nov 2008 11:17:09 -0000
*************** static TypeName *TableFuncTypeName(List
*** 474,480 ****

      WHEN WHERE WHITESPACE_P WITH WITHOUT WORK WRITE

!     XML_P XMLATTRIBUTES XMLCONCAT XMLELEMENT XMLFOREST XMLPARSE
      XMLPI XMLROOT XMLSERIALIZE

      YEAR_P YES_P
--- 474,480 ----

      WHEN WHERE WHITESPACE_P WITH WITHOUT WORK WRITE

!     XML_P XMLATTRIBUTES XMLELEMENT XMLFOREST XMLPARSE
      XMLPI XMLROOT XMLSERIALIZE

      YEAR_P YES_P
*************** func_expr:    func_name '(' ')'
*** 8620,8629 ****
                      v->location = @1;
                      $$ = (Node *)v;
                  }
-             | XMLCONCAT '(' expr_list ')'
-                 {
-                     $$ = makeXmlExpr(IS_XMLCONCAT, NULL, NIL, $3, @1);
-                 }
              | XMLELEMENT '(' NAME_P ColLabel ')'
                  {
                      $$ = makeXmlExpr(IS_XMLELEMENT, $4, NIL, NIL, @1);
--- 8620,8625 ----
*************** col_name_keyword:
*** 9672,9678 ****
              | VALUES
              | VARCHAR
              | XMLATTRIBUTES
-             | XMLCONCAT
              | XMLELEMENT
              | XMLFOREST
              | XMLPARSE
--- 9668,9673 ----
Index: src/backend/parser/keywords.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.205
diff -u -3 -p -c -r1.205 keywords.c
*** src/backend/parser/keywords.c    27 Oct 2008 09:37:47 -0000    1.205
--- src/backend/parser/keywords.c    17 Nov 2008 11:17:09 -0000
*************** const ScanKeyword ScanKeywords[] = {
*** 414,420 ****
      {"write", WRITE, UNRESERVED_KEYWORD},
      {"xml", XML_P, UNRESERVED_KEYWORD},
      {"xmlattributes", XMLATTRIBUTES, COL_NAME_KEYWORD},
-     {"xmlconcat", XMLCONCAT, COL_NAME_KEYWORD},
      {"xmlelement", XMLELEMENT, COL_NAME_KEYWORD},
      {"xmlforest", XMLFOREST, COL_NAME_KEYWORD},
      {"xmlparse", XMLPARSE, COL_NAME_KEYWORD},
--- 414,419 ----
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.237
diff -u -3 -p -c -r1.237 parse_expr.c
*** src/backend/parser/parse_expr.c    26 Oct 2008 02:46:25 -0000    1.237
--- src/backend/parser/parse_expr.c    17 Nov 2008 11:17:09 -0000
*************** transformXmlExpr(ParseState *pstate, Xml
*** 1706,1715 ****
          newe = transformExpr(pstate, e);
          switch (x->op)
          {
-             case IS_XMLCONCAT:
-                 newe = coerce_to_specific_type(pstate, newe, XMLOID,
-                                                "XMLCONCAT");
-                 break;
              case IS_XMLELEMENT:
                  /* no coercion necessary */
                  break;
--- 1706,1711 ----
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.168
diff -u -3 -p -c -r1.168 parse_target.c
*** src/backend/parser/parse_target.c    7 Oct 2008 01:47:55 -0000    1.168
--- src/backend/parser/parse_target.c    17 Nov 2008 11:17:09 -0000
*************** FigureColnameInternal(Node *node, char *
*** 1399,1407 ****
              /* make SQL/XML functions act like a regular function */
              switch (((XmlExpr *) node)->op)
              {
-                 case IS_XMLCONCAT:
-                     *name = "xmlconcat";
-                     return 2;
                  case IS_XMLELEMENT:
                      *name = "xmlelement";
                      return 2;
--- 1399,1404 ----
Index: src/backend/utils/adt/ruleutils.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.287
diff -u -3 -p -c -r1.287 ruleutils.c
*** src/backend/utils/adt/ruleutils.c    6 Oct 2008 20:29:38 -0000    1.287
--- src/backend/utils/adt/ruleutils.c    17 Nov 2008 11:17:09 -0000
*************** get_rule_expr(Node *node, deparse_contex
*** 4519,4527 ****

                  switch (xexpr->op)
                  {
-                     case IS_XMLCONCAT:
-                         appendStringInfoString(buf, "XMLCONCAT(");
-                         break;
                      case IS_XMLELEMENT:
                          appendStringInfoString(buf, "XMLELEMENT(");
                          break;
--- 4519,4524 ----
*************** get_rule_expr(Node *node, deparse_contex
*** 4586,4592 ****
                          appendStringInfoString(buf, ", ");
                      switch (xexpr->op)
                      {
-                         case IS_XMLCONCAT:
                          case IS_XMLELEMENT:
                          case IS_XMLFOREST:
                          case IS_XMLPI:
--- 4583,4588 ----
Index: src/backend/utils/adt/xml.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/xml.c,v
retrieving revision 1.81
diff -u -3 -p -c -r1.81 xml.c
*** src/backend/utils/adt/xml.c    10 Nov 2008 18:02:20 -0000    1.81
--- src/backend/utils/adt/xml.c    17 Nov 2008 11:17:10 -0000
*************** xmlcomment(PG_FUNCTION_ARGS)
*** 422,448 ****
   * TODO: xmlconcat needs to merge the notations and unparsed entities
   * of the argument values.    Not very important in practice, though.
   */
! xmltype *
! xmlconcat(List *args)
  {
  #ifdef USE_LIBXML
      int            global_standalone = 1;
      xmlChar    *global_version = NULL;
      bool        global_version_no_value = false;
      StringInfoData buf;
!     ListCell   *v;

      initStringInfo(&buf);
!     foreach(v, args)
      {
-         xmltype    *x = DatumGetXmlP(PointerGetDatum(lfirst(v)));
          size_t        len;
          xmlChar    *version;
          int            standalone;
          char       *str;

!         len = VARSIZE(x) - VARHDRSZ;
!         str = text_to_cstring((text *) x);

          parse_xml_decl((xmlChar *) str, &len, &version, NULL, &standalone);

--- 422,449 ----
   * TODO: xmlconcat needs to merge the notations and unparsed entities
   * of the argument values.    Not very important in practice, though.
   */
! static xmltype *
! xmlconcat_internal(int num, xmltype *args[])
  {
  #ifdef USE_LIBXML
      int            global_standalone = 1;
      xmlChar    *global_version = NULL;
      bool        global_version_no_value = false;
      StringInfoData buf;
!     int            i;
!
!     Assert(num > 0);

      initStringInfo(&buf);
!     for (i = 0; i < num; i++)
      {
          size_t        len;
          xmlChar    *version;
          int            standalone;
          char       *str;

!         len = VARSIZE(args[i]) - VARHDRSZ;
!         str = text_to_cstring((text *) args[i]);

          parse_xml_decl((xmlChar *) str, &len, &version, NULL, &standalone);

*************** xmlconcat(List *args)
*** 485,490 ****
--- 486,524 ----
  }


+ Datum
+ xmlconcatv(PG_FUNCTION_ARGS)
+ {
+     ArrayType  *arg = PG_GETARG_ARRAYTYPE_P(0);
+     xmltype       **newargs;
+     int            num;
+     int            i;
+
+     Assert(ARR_NDIM(arg) == 1);
+     Assert(ARR_ELEMTYPE(arg) == XMLOID);
+
+     newargs = palloc(sizeof(*newargs) * ARR_DIMS(arg)[0]);
+
+     num = 0;
+     for (i = ARR_LBOUND(arg)[0]; i < ARR_LBOUND(arg)[0] + ARR_DIMS(arg)[0]; i++)
+     {
+         Datum        el;
+         bool        isnull;
+
+         el = array_ref(arg, 1, &i, -1, -1, false, 'i', &isnull);
+         if (isnull)
+             continue;
+         else
+             newargs[num++] = DatumGetXmlP(el);
+     }
+
+     if (num > 0)
+         PG_RETURN_XML_P(xmlconcat_internal(num, newargs));
+     else
+         PG_RETURN_NULL();
+ }
+
+
  /*
   * XMLAGG support
   */
*************** xmlconcat2(PG_FUNCTION_ARGS)
*** 501,508 ****
      else if (PG_ARGISNULL(1))
          PG_RETURN_XML_P(PG_GETARG_XML_P(0));
      else
!         PG_RETURN_XML_P(xmlconcat(list_make2(PG_GETARG_XML_P(0),
!                                              PG_GETARG_XML_P(1))));
  }


--- 535,547 ----
      else if (PG_ARGISNULL(1))
          PG_RETURN_XML_P(PG_GETARG_XML_P(0));
      else
!     {
!         xmltype *newargs[2];
!
!         newargs[0] = PG_GETARG_XML_P(0);
!         newargs[1] = PG_GETARG_XML_P(1);
!         PG_RETURN_XML_P(xmlconcat_internal(2, newargs));
!     }
  }


Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.528
diff -u -3 -p -c -r1.528 pg_proc.h
*** src/include/catalog/pg_proc.h    14 Nov 2008 00:51:46 -0000    1.528
--- src/include/catalog/pg_proc.h    17 Nov 2008 11:17:10 -0000
*************** DATA(insert OID = 2898 (  xml_recv           P
*** 4193,4198 ****
--- 4193,4200 ----
  DESCR("I/O");
  DATA(insert OID = 2899 (  xml_send           PGNSP PGUID 12 1 0 0 f f t f s 1 17 "142" _null_ _null_ _null_ xml_send
_null__null_ _null_ )); 
  DESCR("I/O");
+ DATA(insert OID = 2328 (  xmlconcat           PGNSP PGUID 12 1 0 142 f f f f i 1 142 "143" "{143}" "{v}" _null_
xmlconcatv_null_ _null_ _null_ )); 
+ DESCR("concatenate a list of XML values");
  DATA(insert OID = 2900 (  xmlconcat2       PGNSP PGUID 12 1 0 0 f f f f i 2 142 "142 142" _null_ _null_ _null_
xmlconcat2_null_ _null_ _null_ )); 
  DESCR("aggregate transition function");
  DATA(insert OID = 2901 (  xmlagg           PGNSP PGUID 12 1 0 0 t f f f i 1 142 "142" _null_ _null_ _null_
aggregate_dummy_null_ _null_ _null_ )); 
Index: src/include/nodes/primnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/primnodes.h,v
retrieving revision 1.143
diff -u -3 -p -c -r1.143 primnodes.h
*** src/include/nodes/primnodes.h    6 Oct 2008 17:39:26 -0000    1.143
--- src/include/nodes/primnodes.h    17 Nov 2008 11:17:10 -0000
*************** typedef struct MinMaxExpr
*** 837,843 ****
   */
  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) */
--- 837,842 ----
Index: src/include/utils/xml.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/xml.h,v
retrieving revision 1.24
diff -u -3 -p -c -r1.24 xml.h
*** src/include/utils/xml.h    4 Apr 2008 08:33:15 -0000    1.24
--- src/include/utils/xml.h    17 Nov 2008 11:17:10 -0000
*************** extern Datum xml_out(PG_FUNCTION_ARGS);
*** 32,37 ****
--- 32,38 ----
  extern Datum xml_recv(PG_FUNCTION_ARGS);
  extern Datum xml_send(PG_FUNCTION_ARGS);
  extern Datum xmlcomment(PG_FUNCTION_ARGS);
+ extern Datum xmlconcatv(PG_FUNCTION_ARGS);
  extern Datum xmlconcat2(PG_FUNCTION_ARGS);
  extern Datum texttoxml(PG_FUNCTION_ARGS);
  extern Datum xmltotext(PG_FUNCTION_ARGS);
*************** typedef enum
*** 63,69 ****
      XML_STANDALONE_OMITTED
  } XmlStandaloneType;

- extern xmltype *xmlconcat(List *args);
  extern xmltype *xmlelement(XmlExprState *xmlExpr, ExprContext *econtext);
  extern xmltype *xmlparse(text *data, XmlOptionType xmloption, bool preserve_whitespace);
  extern xmltype *xmlpi(char *target, text *arg, bool arg_is_null, bool *result_is_null);
--- 64,69 ----
Index: src/test/regress/expected/xml.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/xml.out,v
retrieving revision 1.23
diff -u -3 -p -c -r1.23 xml.out
*** src/test/regress/expected/xml.out    15 Nov 2008 20:52:35 -0000    1.23
--- src/test/regress/expected/xml.out    17 Nov 2008 11:17:10 -0000
*************** SELECT xmlconcat('hello', 'you');
*** 55,63 ****
  (1 row)

  SELECT xmlconcat(1, 2);
! ERROR:  argument of XMLCONCAT must be type xml, not type integer
  LINE 1: SELECT xmlconcat(1, 2);
!                          ^
  SELECT xmlconcat('bad', '<syntax');
  ERROR:  invalid XML content
  LINE 1: SELECT xmlconcat('bad', '<syntax');
--- 55,64 ----
  (1 row)

  SELECT xmlconcat(1, 2);
! ERROR:  function xmlconcat(integer, integer) does not exist
  LINE 1: SELECT xmlconcat(1, 2);
!                ^
! HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
  SELECT xmlconcat('bad', '<syntax');
  ERROR:  invalid XML content
  LINE 1: SELECT xmlconcat('bad', '<syntax');
*************** SELECT table_name, view_definition FROM
*** 435,441 ****
   table_name |                                                      view_definition
                   

------------+----------------------------------------------------------------------------------------------------------------------------
   xmlview1   | SELECT xmlcomment('test'::text) AS xmlcomment;
!  xmlview2   | SELECT XMLCONCAT('hello'::xml, 'you'::xml) AS "xmlconcat";
   xmlview3   | SELECT XMLELEMENT(NAME element, XMLATTRIBUTES(1 AS ":one:", 'deuce' AS two), 'content&') AS
"xmlelement";
   xmlview4   | SELECT XMLELEMENT(NAME employee, XMLFOREST(emp.name AS name, emp.age AS age, emp.salary AS pay)) AS
"xmlelement"FROM emp; 
   xmlview5   | SELECT XMLPARSE(CONTENT '<abc>x</abc>'::text STRIP WHITESPACE) AS "xmlparse";
--- 436,442 ----
   table_name |                                                      view_definition
                   

------------+----------------------------------------------------------------------------------------------------------------------------
   xmlview1   | SELECT xmlcomment('test'::text) AS xmlcomment;
!  xmlview2   | SELECT xmlconcat(VARIADIC ARRAY['hello'::xml, 'you'::xml]) AS xmlconcat;
   xmlview3   | SELECT XMLELEMENT(NAME element, XMLATTRIBUTES(1 AS ":one:", 'deuce' AS two), 'content&') AS
"xmlelement";
   xmlview4   | SELECT XMLELEMENT(NAME employee, XMLFOREST(emp.name AS name, emp.age AS age, emp.salary AS pay)) AS
"xmlelement"FROM emp; 
   xmlview5   | SELECT XMLPARSE(CONTENT '<abc>x</abc>'::text STRIP WHITESPACE) AS "xmlparse";