Merge pull request #4 from commandprompt/test/differential-check

Add a differential check, and fix the plxgo Sprintf bug it found

Author: jdatcmd

.gitignore

@@ -7,6 +7,10 @@
 test/results/
 regression.diffs
 regression.out
+# Python bytecode from the test runners
+__pycache__/
+*.pyc
+
 # editor
 *.swp
 *~

CHANGELOG.md

@@ -6,6 +6,27 @@ version in `plx.control` (currently `1.0`).
 
 ## [Unreleased]
 
+### Fixed
+
+- `plxgo`: `fmt.Sprintf` in expression position passed its Go format string
+  straight into SQL `format()`, so any verb other than `%s` raised
+  `unrecognized format() type specifier` when the function was called. The
+  transpile succeeded and the failure only appeared at run time, which made
+  `fmt.Sprintf("%d", n)`, the ordinary way to format an integer in Go, produce
+  a function that could not run. Go's verbs are now rewritten to the `%s` that
+  `format()` understands, keeping a `-` flag and a width and dropping the
+  flags and precision `format()` has no equivalent for. Found by the new
+  differential check.
+
+### Added
+
+- `make differentialcheck` (`test/differential.py`): each case is one program
+  written as a plpgsql reference and once per dialect, called with the same
+  arguments and required to agree with the reference on both values and error
+  SQLSTATEs. Intended divergences are recorded per case with a reason and
+  reported separately, and the check fails if a recorded divergence stops
+  happening, so a documented limitation cannot outlive its documentation.
+
 ### Changed
 
 - Internal refactor of the transpiler behind a `PlxSurface.parse_body` vtable

Makefile

@@ -6,7 +6,8 @@ EXTENSION = plx
 DATA = plx--1.0.sql plx--1.1.sql plx--1.1.1.sql plx--1.2.sql plx--1.2.1.sql plx--1.2.2.sql plx--1.3.0.sql plx--1.3.1.sql plx--1.0--1.1.sql plx--1.1--1.1.1.sql plx--1.1.1--1.2.sql plx--1.2--1.2.1.sql plx--1.2.1--1.2.2.sql plx--1.2.2--1.3.0.sql plx--1.3.0--1.3.1.sql
 
 # pg_regress suite (make installcheck). test/run_corpus.py is an additional
-# Ruby corpus runner.
+# Ruby corpus runner, and test/differential.py checks each dialect against a
+# plpgsql reference (make differentialcheck).
 REGRESS = plxruby plxphp plxjs plxpython3 plxcobol plxplsql plxts plxtsql plxgo plx_features plx_output plx_strbuild plx_errors
 REGRESS_OPTS = --inputdir=test --outputdir=test
 
@@ -18,3 +19,12 @@ include $(PGXS)
 # PGXS does not track header dependencies; declare them so a header change
 # rebuilds every object (the shared enum/struct layout must stay consistent).
 $(OBJS): src/plx.h src/plx_int.h src/plx_engine.h
+
+PLX_PYTHON ?= python3
+
+# Differential check: run every dialect's version of a program against the
+# plpgsql the same program would have been written in, and require them to
+# agree. Needs the extension installed and a running server, like installcheck.
+.PHONY: differentialcheck
+differentialcheck:
+	$(PLX_PYTHON) test/differential.py

doc/ARCHITECTURE.md

@@ -158,6 +158,39 @@ load a full interpreter into the backend. The transpiler is C that parses
 untrusted input at `CREATE FUNCTION` time; it has a recursion-depth limit and is
 fuzzed (see `test/fuzz.py`).
 
+## Testing
+
+Four layers, each answering a different question.
+
+`make installcheck` runs the pg_regress suite in `test/sql`, one file per
+dialect plus the shared feature, output, string-builder and error files. This
+is the record of what each dialect accepts and what plpgsql it produces.
+
+`test/run_corpus.py` replays a corpus of Ruby programs and compares their
+output to a recorded expectation, including the programs that are supposed to
+be rejected.
+
+`test/fuzz.py` feeds mutated and pathological sources to every front end. It
+asserts nothing about the output; it asserts that the transpiler does not
+crash, hang, or exhaust memory on input written to break it.
+
+`make differentialcheck` runs `test/differential.py`, which answers the
+question the other three cannot. Each case is one small program written once
+as a plpgsql reference and once per dialect, then called with the same
+arguments. The reference is the plpgsql a PostgreSQL developer would have
+written for the same logic, so it acts as an oracle: plx claims a dialect body
+transpiles to plpgsql that behaves the same way, and a disagreement is a defect
+rather than a difference of opinion. Values are compared by their text form,
+with NULL distinguished from the empty string, and a case that raises must
+raise the same SQLSTATE.
+
+Some divergences are intended, and a case records them in a `documented` entry
+naming the dialects, the calls, and the reason. Those are reported separately
+rather than failing. The check also fails when a recorded divergence stops
+happening, so a limitation cannot quietly outlive the documentation that
+describes it. Where a case cannot cover a dialect, the run prints the gap
+instead of passing over it.
+
 ## Related documents
 
 - [TRANSPILER.md](TRANSPILER.md): the original transpiler design specification.

doc/LIMITATIONS.md

@@ -117,6 +117,14 @@ 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
+  example `to_hex`) where the representation matters.
 
 ### plxcobol ([chapter](plxcobol.md))
 

doc/plxgo.md

@@ -136,9 +136,22 @@ to `a[(i) + 1]`. Indexing therefore follows Go semantics: `a[0]` is the first
 element, and `len(a)`, `for i := range a`, and `a[i]` all agree on 0-based
 positions. Go slice expressions (`a[i:j]`) are not translated.
 
-`fmt.Println`/`fmt.Printf` raise a `NOTICE` with one `%` placeholder per value
-argument (space-separated); a `Printf`/`Sprintf` format string's literal text and
-directives are not reproduced, since SQL `RAISE` has no printf verbs.
+`fmt.Println`/`fmt.Printf` used as a statement raise a `NOTICE` with one `%`
+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`.
+A doubled `%%` stays a literal `%`, and a `%` that starts no directive is
+passed through as a literal percent.
 
 ### Types (in declarations)
 

src/plx_dialect_go.c

@@ -410,6 +410,85 @@ go_emit_str(GoTok *tk, StringInfo out)
 	appendStringInfoChar(out, '\'');
 }
 
+/* emit a Go format string as one SQL format() accepts.
+
+ * Go's verbs all render their operand as text, which is what format()'s %s
+ * does, so every directive becomes %s. format() takes only a '-' flag and a
+ * width, so Go's other flags and its precision field are dropped: the operand
+ * still appears, just without the padding Go would have applied. A doubled
+ * A doubled %% passes through, and a '%' that starts no directive is escaped
+ * to %%, so it reaches the caller as a literal percent instead of tripping
+ * format() at run time.
+ */
+static void
+go_emit_format_str(GoTok *tk, StringInfo out)
+{
+	StringInfoData raw;
+	int			i;
+
+	initStringInfo(&raw);
+	go_emit_str(tk, &raw);		/* a quoted SQL literal, escapes decoded */
+
+	for (i = 0; i < raw.len; i++)
+	{
+		char		c = raw.data[i];
+		bool		dash = false;
+		int			j,
+					wstart,
+					wlen;
+
+		if (c != '%')
+		{
+			appendStringInfoChar(out, c);
+			continue;
+		}
+		if (i + 1 < raw.len && raw.data[i + 1] == '%')
+		{
+			appendStringInfoString(out, "%%");
+			i++;
+			continue;
+		}
+
+		j = i + 1;
+		while (j < 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 < raw.len && raw.data[j] >= '0' && raw.data[j] <= '9')
+			j++;
+		wlen = j - wstart;
+		if (j < raw.len && raw.data[j] == '.')
+		{
+			j++;
+			while (j < raw.len && raw.data[j] >= '0' && raw.data[j] <= '9')
+				j++;
+		}
+		if (j >= raw.len ||
+			!((raw.data[j] >= 'a' && raw.data[j] <= 'z') ||
+			  (raw.data[j] >= 'A' && raw.data[j] <= 'Z')))
+		{
+			/* starts no directive: escape it, since a lone '%' is itself
+			 * an error to format() */
+			appendStringInfoString(out, "%%");
+			continue;
+		}
+
+		appendStringInfoChar(out, '%');
+		if (dash)
+			appendStringInfoChar(out, '-');
+		if (wlen > 0)
+			appendBinaryStringInfo(out, raw.data + wstart, wlen);
+		appendStringInfoChar(out, 's');
+		i = j;
+	}
+	pfree(raw.data);
+}
+
 /* map a Go base type name to a PostgreSQL type; NULL if unknown */
 static const char *
 go_base_type(const char *s, int len)
@@ -580,6 +659,22 @@ go_emit_call(Go *g, int i, int b, StringInfo out, int *ni)
 		close = go_match(g, lp, b);
 		if (close < 0)
 			return false;
+
+		/* fmt.Sprintf in expression position: reproduce the format string,
+		 * rewriting Go's verbs into the ones SQL format() understands */
+		if (go_ci(pkg, "fmt") && go_ci(meth, "Sprintf") &&
+			close > lp + 1 && g->t[lp + 1].kind == GO_STR)
+		{
+			go_sp(out);
+			appendStringInfoString(out, "format(");
+			go_emit_format_str(&g->t[lp + 1], out);
+			if (lp + 2 < close)
+				go_emit_range(g, lp + 2, close, out);
+			appendStringInfoChar(out, ')');
+			*ni = close + 1;
+			return true;
+		}
+
 		for (k = 0; r[k].pkg; k++)
 			if ((int) strlen(r[k].pkg) == pkg->len &&
 				strncmp(r[k].pkg, pkg->s, pkg->len) == 0 &&