Fix concat_ws() to not insert a separator after leading NULL argument(s).

Per bug #6181 from Itagaki Takahiro.  Also do some marginal code cleanup
and improve error handling.

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index cdeee6b8e7efe6d33611700a3a8ca12e7b009fdc..a406794e4d01ad3df17ba3b07ec3d0994c8b0bf3 100644 (file)
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3622,11 +3622,19 @@ string_agg_finalfn(PG_FUNCTION_ARGS)
                PG_RETURN_NULL();
 }
 
+/*
+ * Implementation of both concat() and concat_ws().
+ *
+ * sepstr/seplen describe the separator.  argidx is the first argument
+ * to concatenate (counting from zero).
+ */
 static text *
-concat_internal(const char *sepstr, int seplen, int argidx, FunctionCallInfo fcinfo)
+concat_internal(const char *sepstr, int seplen, int argidx,
+                               FunctionCallInfo fcinfo)
 {
-       StringInfoData str;
        text       *result;
+       StringInfoData str;
+       bool            first_arg = true;
        int                     i;
 
        initStringInfo(&str);
@@ -3635,17 +3643,21 @@ concat_internal(const char *sepstr, int seplen, int argidx, FunctionCallInfo fci
        {
                if (!PG_ARGISNULL(i))
                {
+                       Datum           value = PG_GETARG_DATUM(i);
                        Oid                     valtype;
-                       Datum           value;
                        Oid                     typOutput;
                        bool            typIsVarlena;
 
-                       if (i > argidx)
+                       /* add separator if appropriate */
+                       if (first_arg)
+                               first_arg = false;
+                       else
                                appendBinaryStringInfo(&str, sepstr, seplen);
 
-                       /* append n-th value */
-                       value = PG_GETARG_DATUM(i);
+                       /* call the appropriate type output function, append the result */
                        valtype = get_fn_expr_argtype(fcinfo->flinfo, i);
+                       if (!OidIsValid(valtype))
+                               elog(ERROR, "could not determine data type of concat() input");
                        getTypeOutputInfo(valtype, &typOutput, &typIsVarlena);
                        appendStringInfoString(&str,
                                                                   OidOutputFunctionCall(typOutput, value));
@@ -3664,12 +3676,12 @@ concat_internal(const char *sepstr, int seplen, int argidx, FunctionCallInfo fci
 Datum
 text_concat(PG_FUNCTION_ARGS)
 {
-       PG_RETURN_TEXT_P(concat_internal(NULL, 0, 0, fcinfo));
+       PG_RETURN_TEXT_P(concat_internal("", 0, 0, fcinfo));
 }
 
 /*
- * Concatenate all but first argument values with separators. The first
- * parameter is used as a separator. NULL arguments are ignored.
+ * Concatenate all but first argument value with separators. The first
+ * parameter is used as the separator. NULL arguments are ignored.
  */
 Datum
 text_concat_ws(PG_FUNCTION_ARGS)
@@ -3682,8 +3694,8 @@ text_concat_ws(PG_FUNCTION_ARGS)
 
        sep = PG_GETARG_TEXT_PP(0);
 
-       PG_RETURN_TEXT_P(concat_internal(
-                                          VARDATA_ANY(sep), VARSIZE_ANY_EXHDR(sep), 1, fcinfo));
+       PG_RETURN_TEXT_P(concat_internal(VARDATA_ANY(sep), VARSIZE_ANY_EXHDR(sep),
+                                                                        1, fcinfo));
 }
 
 /*