Lower plxgo fmt.Sprintf to concatenation so NULL propagates

doc/LIMITATIONS.md

@@ -40,9 +40,9 @@ These follow from running as plpgsql and are not specific to any one dialect.
   concatenated as-is, so the whole string becomes NULL the way SQL `||` does.
   Two exceptions. A message being built for `RAISE` keeps each interpolated
   value as an empty string, so one NULL cannot swallow the message. And
-  `plxgo`'s `fmt.Sprintf` and `plxcobol`'s `STRING-APPEND` do not propagate,
-  because they build their string through SQL `format()` and the `plx_strbuild`
-  accumulator respectively, neither of which treats a NULL operand as NULL.
+  `plxcobol`'s `STRING-APPEND` does not propagate, because it builds its string
+  through the `plx_strbuild` accumulator, whose append treats a NULL as nothing
+  to append by design.
 - **String concatenation with `+` is not string concatenation.** In every dialect
   `+` is SQL numeric addition. Use the dialect's string form: interpolation
   (Ruby/PHP/Python/JS/TS), `||`, or `CONCAT(...)`.
@@ -130,13 +130,12 @@ The per-dialect chapter is authoritative; this is a quick reference.
 - `+` for string concatenation (use `||` or build a slice and `array_to_string`).
 - Only a subset of `fmt`/`strings`/`math`/`strconv` is mapped; other calls pass
   through and must be valid PostgreSQL functions.
-- `fmt.Sprintf` renders every operand in its SQL text form, because each Go verb
-  becomes `format()`'s `%s`. A `-` flag and a width are kept, so `%-8d` still
-  pads. Go's other flags and its precision field are dropped, so `%.2f` prints
-  the operand in full rather than rounding it. The verbs that change an
-  operand's representation rather than its padding do not do so here: `%x`,
-  `%o`, `%b`, `%e` and `%q` all produce the same text `%s` would, so
-  `fmt.Sprintf("%x", 255)` yields `255` and not `ff`. Convert explicitly (for
+- `fmt.Sprintf` renders every operand in its SQL text form. A `-` flag and a
+  width are kept, so `%-8d` still pads. Go's other flags and its precision field
+  are dropped, so `%.2f` prints the operand in full rather than rounding it. The
+  verbs that change an operand's representation rather than its padding do not
+  do so either: `%x`, `%o`, `%b`, `%e` and `%q` all produce the operand's text,
+  so `fmt.Sprintf("%x", 255)` yields `255` and not `ff`. Convert explicitly (for
   example `to_hex`) where the representation matters.
 
 ### plxcobol ([chapter](plxcobol.md))

doc/plxgo.md

@@ -141,15 +141,28 @@ placeholder per value argument (space-separated); there the format string's
 literal text and directives are not reproduced, since SQL `RAISE` has no printf
 verbs.
 
-`fmt.Sprintf` in an expression becomes SQL `format()`, and there the format
-string is reproduced. Every Go verb renders its operand as text, which is what
-`format()`'s `%s` does, so `%d`, `%v`, `%f`, `%q` and the rest all become `%s`.
-A `-` flag and a width are kept, so `%-8d` still pads to eight columns. Go's
-other flags and its precision field have no `format()` equivalent and are
-dropped, so `%.2f` prints the operand's full text rather than rounding it to
-two decimal places. Verbs that change an operand's representation rather than
-its padding are affected the same way: `%x`, `%o`, `%b`, `%e` and `%q` all
-render what `%s` would, so `fmt.Sprintf("%x", 255)` yields `255` and not `ff`.
+`fmt.Sprintf` in an expression becomes a SQL concatenation, with the format
+string's literal text reproduced between the operands:
+
+```go
+fmt.Sprintf("user %s has %d items", nm, n)
+```
+```sql
+'user ' || (nm)::text || ' has ' || (n)::text || ' items'
+```
+
+Concatenating rather than calling `format()` is what lets a NULL operand
+propagate, so the whole string is NULL instead of the operand quietly rendering
+as empty. A message built for `panic` is the exception and keeps each operand's
+empty-string fallback, so one NULL cannot swallow the message.
+
+Every verb renders its operand as text. A `-` flag and a width are kept and
+become `rpad`/`lpad`, so `%-8d` still pads to eight columns. Go's other flags
+and its precision field are dropped, so `%.2f` prints the operand's full text
+rather than rounding it to two decimal places. Verbs that change an operand's
+representation rather than its padding are affected the same way: `%x`, `%o`,
+`%b`, `%e` and `%q` all render the operand's text, so `fmt.Sprintf("%x", 255)`
+yields `255` and not `ff`.
 A doubled `%%` stays a literal `%`, and a `%` that starts no directive is
 passed through as a literal percent.
 

src/plx_dialect_go.c

@@ -489,6 +489,184 @@ go_emit_format_str(GoTok *tk, StringInfo out)
 	pfree(raw.data);
 }
 
+/* fmt.Sprintf with a literal format string, lowered to a || chain.
+
+ * format() renders a NULL operand as the empty string and cannot be made to do
+ * otherwise, so a Sprintf lowered to format() silently turns a missing value
+ * into a plausible-looking one. Concatenating instead lets NULL propagate the
+ * way SQL || does. Width survives as lpad/rpad; the operand still renders as
+ * text, so Go's representation verbs are unaffected by the change.
+ *
+ * A diagnostic message keeps a COALESCE per operand, so one NULL cannot swallow
+ * the message. Returns false when the directives and arguments do not line up,
+ * leaving the caller to fall back to format().
+ */
+static bool
+go_emit_sprintf_concat(Go *g, int lp, int close, StringInfo out)
+{
+	StringInfoData raw,
+				lit;
+	int			as[16],
+				ae[16];
+	int			nargs = 0,
+				ndir = 0,
+				ai = 0,
+				i,
+				d,
+				seg;
+	bool		first = true;
+	bool		diag = g->cx->diag_msg;
+
+	/* arguments follow the format literal and its comma */
+	i = lp + 2;
+	if (i < close && g->t[i].kind == GO_COMMA)
+	{
+		i++;
+		seg = i;
+		d = 0;
+		for (; i < close; i++)
+		{
+			GoKind		k = g->t[i].kind;
+
+			if (k == GO_LP || k == GO_LBRACK || k == GO_LBRACE)
+				d++;
+			else if (k == GO_RP || k == GO_RBRACK || k == GO_RBRACE)
+				d--;
+			else if (k == GO_COMMA && d == 0)
+			{
+				if (nargs >= 16)
+					return false;
+				as[nargs] = seg;
+				ae[nargs] = i;
+				nargs++;
+				seg = i + 1;
+			}
+		}
+		if (seg < close)
+		{
+			if (nargs >= 16)
+				return false;
+			as[nargs] = seg;
+			ae[nargs] = close;
+			nargs++;
+		}
+	}
+
+	initStringInfo(&raw);
+	go_emit_str(&g->t[lp + 1], &raw);		/* quoted, escapes decoded */
+
+	/* count directives so a mismatch can fall back before emitting anything */
+	for (i = 1; i + 1 < raw.len; i++)
+	{
+		if (raw.data[i] != '%')
+			continue;
+		if (raw.data[i + 1] == '%')
+		{
+			i++;
+			continue;
+		}
+		ndir++;
+	}
+	if (ndir != nargs)
+	{
+		pfree(raw.data);
+		return false;
+	}
+
+	initStringInfo(&lit);
+	go_sp(out);
+	appendStringInfoChar(out, '(');
+	for (i = 1; i + 1 < raw.len; i++)
+	{
+		char		c = raw.data[i];
+		bool		dash = false;
+		int			j,
+					wstart,
+					wlen;
+
+		if (c != '%')
+		{
+			appendStringInfoChar(&lit, c);
+			continue;
+		}
+		if (raw.data[i + 1] == '%')
+		{
+			appendStringInfoChar(&lit, '%');	/* a literal percent needs no
+												 * escape outside format() */
+			i++;
+			continue;
+		}
+
+		j = i + 1;
+		while (j + 1 < raw.len && (raw.data[j] == '-' || raw.data[j] == '+' ||
+								   raw.data[j] == ' ' || raw.data[j] == '#' ||
+								   raw.data[j] == '0'))
+		{
+			if (raw.data[j] == '-')
+				dash = true;
+			j++;
+		}
+		wstart = j;
+		while (j + 1 < raw.len && raw.data[j] >= '0' && raw.data[j] <= '9')
+			j++;
+		wlen = j - wstart;
+		if (j + 1 < raw.len && raw.data[j] == '.')
+		{
+			j++;
+			while (j + 1 < raw.len && raw.data[j] >= '0' && raw.data[j] <= '9')
+				j++;
+		}
+
+		/* flush the literal run before the value */
+		if (lit.len > 0)
+		{
+			if (!first)
+				appendStringInfoString(out, " || ");
+			appendStringInfoChar(out, '\'');
+			appendBinaryStringInfo(out, lit.data, lit.len);
+			appendStringInfoChar(out, '\'');
+			resetStringInfo(&lit);
+			first = false;
+		}
+
+		if (!first)
+			appendStringInfoString(out, " || ");
+		if (diag)
+			appendStringInfoString(out, "COALESCE(");
+		if (wlen > 0)
+			appendStringInfoString(out, dash ? "rpad(" : "lpad(");
+		appendStringInfoChar(out, '(');
+		go_emit_range(g, as[ai], ae[ai], out);
+		appendStringInfoString(out, ")::text");
+		if (wlen > 0)
+		{
+			appendStringInfoString(out, ", ");
+			appendBinaryStringInfo(out, raw.data + wstart, wlen);
+			appendStringInfoChar(out, ')');
+		}
+		if (diag)
+			appendStringInfoString(out, ", '')");
+		first = false;
+		ai++;
+		i = j;
+	}
+	if (lit.len > 0)
+	{
+		if (!first)
+			appendStringInfoString(out, " || ");
+		appendStringInfoChar(out, '\'');
+		appendBinaryStringInfo(out, lit.data, lit.len);
+		appendStringInfoChar(out, '\'');
+		first = false;
+	}
+	if (first)								/* an empty format string */
+		appendStringInfoString(out, "''");
+	appendStringInfoChar(out, ')');
+	pfree(raw.data);
+	pfree(lit.data);
+	return true;
+}
+
 /* map a Go base type name to a PostgreSQL type; NULL if unknown */
 static const char *
 go_base_type(const char *s, int len)
@@ -665,6 +843,11 @@ go_emit_call(Go *g, int i, int b, StringInfo out, int *ni)
 		if (go_ci(pkg, "fmt") && go_ci(meth, "Sprintf") &&
 			close > lp + 1 && g->t[lp + 1].kind == GO_STR)
 		{
+			if (go_emit_sprintf_concat(g, lp, close, out))
+			{
+				*ni = close + 1;
+				return true;
+			}
 			go_sp(out);
 			appendStringInfoString(out, "format(");
 			go_emit_format_str(&g->t[lp + 1], out);
@@ -1176,7 +1359,9 @@ go_simple(Go *g, int ind, int stop)
 			if (close > lp + 1)
 			{
 				appendStringInfoString(&g->cx->out, "RAISE EXCEPTION '%',");
+				g->cx->diag_msg = true;
 				go_emit_range(g, lp + 1, close, &g->cx->out);
+				g->cx->diag_msg = false;
 				appendStringInfoString(&g->cx->out, ";\n");
 			}
 			else

test/differential_cases.py

@@ -391,16 +391,16 @@
   RETURN 'user ' || @nm || ' has ' || @n || ' items';
 """,
         },
-        # Interpolation now propagates NULL in every dialect that has it, so
-        # the reference holds. plxgo builds this string through SQL format()
-        # and plxcobol through the plx_strbuild accumulator, neither of which
-        # propagates, so those two still diverge.
+        # Interpolation propagates NULL in every dialect that has it, so the
+        # reference holds. plxcobol builds this string through the plx_strbuild
+        # accumulator, whose append is deliberately not strict, so it still
+        # renders a NULL operand as empty.
         "documented": [
             {
-                "dialects": ["plxgo", "plxcobol"],
+                "dialects": ["plxcobol"],
                 "calls": ["NULL, 3", "'bob', NULL"],
-                "reason": "format() and the strbuild accumulator render a NULL "
-                          "operand as empty (doc/LIMITATIONS.md)",
+                "reason": "the plx_strbuild accumulator appends a NULL as "
+                          "nothing by design (doc/LIMITATIONS.md)",
             },
         ],
     },

test/expected/plxgo.out

@@ -333,3 +333,22 @@ SELECT g_sprintf_repr(255) AS repr_verbs_are_text;
  255|255|255|255
 (1 row)
 
+-- fmt.Sprintf concatenates, so a NULL operand propagates rather than rendering
+-- as an empty string
+CREATE FUNCTION g_sprintf_null(nm text, n int) RETURNS text LANGUAGE plxgo AS $$
+	return fmt.Sprintf("user %s has %d items", nm, n)
+$$;
+SELECT g_sprintf_null('bob', 3) AS ok,
+       g_sprintf_null(NULL, 3) IS NULL AS null_name,
+       g_sprintf_null('bob', NULL) IS NULL AS null_count;
+          ok          | null_name | null_count 
+----------------------+-----------+------------
+ user bob has 3 items | t         | t
+(1 row)
+
+-- but a panic message keeps its literal text when an operand is NULL
+CREATE FUNCTION g_panic_msg(who text) RETURNS int LANGUAGE plxgo AS $$
+	panic(fmt.Sprintf("bad user %s here", who))
+$$;
+DO $d$ BEGIN PERFORM g_panic_msg(NULL);
+EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'caught: [%]', SQLERRM; END $d$;

test/sql/plxgo.sql

@@ -230,3 +230,19 @@ CREATE FUNCTION g_sprintf_repr(n int) RETURNS text LANGUAGE plxgo AS $$
 	return fmt.Sprintf("%x|%o|%b|%q", n, n, n, n)
 $$;
 SELECT g_sprintf_repr(255) AS repr_verbs_are_text;
+
+-- fmt.Sprintf concatenates, so a NULL operand propagates rather than rendering
+-- as an empty string
+CREATE FUNCTION g_sprintf_null(nm text, n int) RETURNS text LANGUAGE plxgo AS $$
+	return fmt.Sprintf("user %s has %d items", nm, n)
+$$;
+SELECT g_sprintf_null('bob', 3) AS ok,
+       g_sprintf_null(NULL, 3) IS NULL AS null_name,
+       g_sprintf_null('bob', NULL) IS NULL AS null_count;
+
+-- but a panic message keeps its literal text when an operand is NULL
+CREATE FUNCTION g_panic_msg(who text) RETURNS int LANGUAGE plxgo AS $$
+	panic(fmt.Sprintf("bad user %s here", who))
+$$;
+DO $d$ BEGIN PERFORM g_panic_msg(NULL);
+EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'caught: [%]', SQLERRM; END $d$;