Skip to content

Commit b3affbf

Browse files
plxclaude
andcommitted
Address review of the Sprintf fix and harden the differential probe
Three defects found reviewing the previous commit against its own intent. The Sprintf rewriter passed a '%' that starts no directive straight through. The point of the change was that Sprintf should stop producing functions that fail when called, but a lone '%' is itself an error to format(), so fmt.Sprintf("100%") still raised "unterminated format() type specifier" at run time. It is now escaped to %% and reaches the caller as a literal percent. The documentation understated what dropping a verb costs. Saying only that flags and precision are dropped implies the difference is padding, but the verbs that change an operand's representation are affected too: %x, %o, %b, %e and %q all render what %s renders, so fmt.Sprintf("%x", 255) yields 255 rather than ff. That is a wrong value rather than a wrong width, which is worse than an error because it is silent, so both doc/plxgo.md and doc/LIMITATIONS.md now say so and point at an explicit conversion. The differential probe returned values verbatim on one output line per probe. A value containing a newline, or a blank value dropped by the runner's filter, would shift every later result onto the wrong probe. Counts usually catch that, but a two-line value and a blank value in the same run cancel out and the mis-mapping is silent. The probe now returns a sentinel for the empty string and escapes newlines, so one probe is always exactly one line, and the runner no longer discards blank lines. Regression cases added for the bare percent, a percent followed by punctuation, and the representation-changing verbs. Verified on PostgreSQL 18.4: clean build with no warnings, 13/13 installcheck, differentialcheck clean at 374 matching and 14 documented, and the check still exits 1 on an injected defect after the probe change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1faeebf commit b3affbf

6 files changed

Lines changed: 74 additions & 8 deletions

File tree

doc/LIMITATIONS.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,14 @@ The per-dialect chapter is authoritative; this is a quick reference.
117117
- `+` for string concatenation (use `||` or build a slice and `array_to_string`).
118118
- Only a subset of `fmt`/`strings`/`math`/`strconv` is mapped; other calls pass
119119
through and must be valid PostgreSQL functions.
120-
- `fmt.Sprintf` keeps a `-` flag and a width, but drops Go's other flags and its
121-
precision field, since SQL `format()` has no equivalent. `%.2f` therefore
122-
prints the operand in full instead of rounding it to two decimal places.
120+
- `fmt.Sprintf` renders every operand in its SQL text form, because each Go verb
121+
becomes `format()`'s `%s`. A `-` flag and a width are kept, so `%-8d` still
122+
pads. Go's other flags and its precision field are dropped, so `%.2f` prints
123+
the operand in full rather than rounding it. The verbs that change an
124+
operand's representation rather than its padding do not do so here: `%x`,
125+
`%o`, `%b`, `%e` and `%q` all produce the same text `%s` would, so
126+
`fmt.Sprintf("%x", 255)` yields `255` and not `ff`. Convert explicitly (for
127+
example `to_hex`) where the representation matters.
123128

124129
### plxcobol ([chapter](plxcobol.md))
125130

doc/plxgo.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,11 @@ string is reproduced. Every Go verb renders its operand as text, which is what
147147
A `-` flag and a width are kept, so `%-8d` still pads to eight columns. Go's
148148
other flags and its precision field have no `format()` equivalent and are
149149
dropped, so `%.2f` prints the operand's full text rather than rounding it to
150-
two decimal places. A doubled `%%` stays a literal `%`.
150+
two decimal places. Verbs that change an operand's representation rather than
151+
its padding are affected the same way: `%x`, `%o`, `%b`, `%e` and `%q` all
152+
render what `%s` would, so `fmt.Sprintf("%x", 255)` yields `255` and not `ff`.
153+
A doubled `%%` stays a literal `%`, and a `%` that starts no directive is
154+
passed through as a literal percent.
151155

152156
### Types (in declarations)
153157

src/plx_dialect_go.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,9 @@ go_emit_str(GoTok *tk, StringInfo out)
416416
* does, so every directive becomes %s. format() takes only a '-' flag and a
417417
* width, so Go's other flags and its precision field are dropped: the operand
418418
* still appears, just without the padding Go would have applied. A doubled
419-
* %% passes through, and a '%' that starts no directive is left alone.
419+
* A doubled %% passes through, and a '%' that starts no directive is escaped
420+
* to %%, so it reaches the caller as a literal percent instead of tripping
421+
* format() at run time.
420422
*/
421423
static void
422424
go_emit_format_str(GoTok *tk, StringInfo out)
@@ -470,7 +472,9 @@ go_emit_format_str(GoTok *tk, StringInfo out)
470472
!((raw.data[j] >= 'a' && raw.data[j] <= 'z') ||
471473
(raw.data[j] >= 'A' && raw.data[j] <= 'Z')))
472474
{
473-
appendStringInfoChar(out, c); /* not a directive */
475+
/* starts no directive: escape it, since a lone '%' is itself
476+
* an error to format() */
477+
appendStringInfoString(out, "%%");
474478
continue;
475479
}
476480

test/differential.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ def psql(sql, stop_on_error=False):
4949
IF r IS NULL THEN
5050
RETURN '<NULL>';
5151
END IF;
52-
RETURN r;
52+
IF r = '' THEN
53+
RETURN '<EMPTY>';
54+
END IF;
55+
/* One probe must occupy exactly one output line: an embedded newline, or a
56+
* value that is blank, would shift every later result onto the wrong probe. */
57+
RETURN replace(replace(r, E'\r', '\\r'), E'\n', '\\n');
5358
EXCEPTION WHEN OTHERS THEN
5459
RETURN 'ERROR ' || SQLSTATE;
5560
END;
@@ -96,7 +101,7 @@ def probe_all(created_ok):
96101
if not selects:
97102
return {}
98103
out, err, _ = psql("\n".join(s + ";" for s in selects))
99-
lines = [ln for ln in out.splitlines() if ln != ""]
104+
lines = out.splitlines()
100105
if len(lines) != len(keys):
101106
sys.stderr.write("probe count mismatch: %d results for %d probes\n%s\n"
102107
% (len(lines), len(keys), err))

test/expected/plxgo.out

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,3 +303,33 @@ SELECT g_sprintf_prec(1.5) AS prec;
303303
1.5| 1.5
304304
(1 row)
305305

306+
-- a '%' that starts no directive reaches the caller as a literal percent
307+
-- rather than tripping format() at run time
308+
CREATE FUNCTION g_sprintf_bare_pct() RETURNS text LANGUAGE plxgo AS $$
309+
return fmt.Sprintf("100%")
310+
$$;
311+
SELECT g_sprintf_bare_pct() AS should_be_100_pct;
312+
should_be_100_pct
313+
-------------------
314+
100%
315+
(1 row)
316+
317+
CREATE FUNCTION g_sprintf_pct_punct(n int) RETURNS text LANGUAGE plxgo AS $$
318+
return fmt.Sprintf("%d%!", n)
319+
$$;
320+
SELECT g_sprintf_pct_punct(50) AS pct_then_punct;
321+
pct_then_punct
322+
----------------
323+
50%!
324+
(1 row)
325+
326+
-- verbs that change an operand's representation render what %s renders
327+
CREATE FUNCTION g_sprintf_repr(n int) RETURNS text LANGUAGE plxgo AS $$
328+
return fmt.Sprintf("%x|%o|%b|%q", n, n, n, n)
329+
$$;
330+
SELECT g_sprintf_repr(255) AS repr_verbs_are_text;
331+
repr_verbs_are_text
332+
---------------------
333+
255|255|255|255
334+
(1 row)
335+

test/sql/plxgo.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,3 +212,21 @@ CREATE FUNCTION g_sprintf_prec(x float8) RETURNS text LANGUAGE plxgo AS $$
212212
return fmt.Sprintf("%.2f|%8.3f", x, x)
213213
$$;
214214
SELECT g_sprintf_prec(1.5) AS prec;
215+
216+
-- a '%' that starts no directive reaches the caller as a literal percent
217+
-- rather than tripping format() at run time
218+
CREATE FUNCTION g_sprintf_bare_pct() RETURNS text LANGUAGE plxgo AS $$
219+
return fmt.Sprintf("100%")
220+
$$;
221+
SELECT g_sprintf_bare_pct() AS should_be_100_pct;
222+
223+
CREATE FUNCTION g_sprintf_pct_punct(n int) RETURNS text LANGUAGE plxgo AS $$
224+
return fmt.Sprintf("%d%!", n)
225+
$$;
226+
SELECT g_sprintf_pct_punct(50) AS pct_then_punct;
227+
228+
-- verbs that change an operand's representation render what %s renders
229+
CREATE FUNCTION g_sprintf_repr(n int) RETURNS text LANGUAGE plxgo AS $$
230+
return fmt.Sprintf("%x|%o|%b|%q", n, n, n, n)
231+
$$;
232+
SELECT g_sprintf_repr(255) AS repr_verbs_are_text;

0 commit comments

Comments
 (0)