diff --git a/CHANGELOG.md b/CHANGELOG.md index 68e82ea4..efee4d80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,36 @@ All notable changes to EigenScript are documented here. `str of` over the hard doubles and the exact-integer band, validated with a planted fault in each half (the precision escalation and the integer bound are independently load-bearing). +- **DB results carry their SQL type, and DB failures raise (#887, + #888).** Two defects in one function, both silent. `db_query_json` + emitted every column as a JSON string, so SQL `false` arrived as + `"f"` — a non-empty string, therefore **truthy** — and + `if row.is_admin:` passed for a non-admin; NULL and `''` were both + `""` with no way to tell them apart; and `9 > 10` held because `'9' > + '1'`. And every failure — syntax error, missing table, revoked + permission, dead connection — returned `[]`, the same value a + successful query over an empty table returns, so a reporting script + kept printing "0 rows" forever after a schema change. + Now: `PQgetisnull` → `null` (checked before the type), `boolean` → + `true`/`false`, the exact-integer and float types → JSON numbers, + everything else → strings, through **one classifier shared with + `db_query_value`** so the two cannot drift. Failures raise a catchable + `io` error carrying libpq's own first line; a genuinely empty result + is still `[]`/`""` and only that. `db_connect` still reports by return + value, so probing for a database needs no `try`. + Two calls documented in `docs/BUILTINS.md`: **`numeric` stays a + string** (arbitrary-precision decimal cannot round-trip through a + binary double — silently rounding money is the defect class this + fixes; `::float8` is the opt-in), and a **`bigint` past 2^53 raises** + naming the column and the fix (`id::text`) rather than rounding a + primary key. `bigint` is a number rather than a string because + `count(*)` and `sum(integer)` return it — leaving it text would have + left the bug unfixed for most real numeric queries. The mapping is a + function of the column's SQL type alone, never of the row's value. + `tests/test_db.eigs` gained the no-connection raise checks (they run + wherever the extension is compiled in — previously the whole file was + effectively inert without a server) plus DB18–DB26 against the live + CI postgres service. - **chart renders 1.5× faster at high point counts (#828).** The series hot loop called `_chart_map` — a fresh 2-element list — per plotted diff --git a/docs/BUILTINS.md b/docs/BUILTINS.md index 1c8bb916..57da6ed5 100644 --- a/docs/BUILTINS.md +++ b/docs/BUILTINS.md @@ -681,10 +681,69 @@ Requires full build with libpq. PostgreSQL client. | Name | Signature | Description | |------|-----------|-------------| -| `db_connect` | `db_connect of null` | Connect via DATABASE_URL env var | -| `db_query_value` | `db_query_value of sql` or `db_query_value of [sql, p1, p2]` | Execute query, return first value with optional params | -| `db_execute` | `db_execute of sql` or `db_execute of [sql, p1, p2]` | Execute command with optional params | -| `db_query_json` | `db_query_json of sql` or `db_query_json of [sql, p1, p2]` | Execute query, return all rows as JSON with optional params | +| `db_connect` | `db_connect of null` | Connect via DATABASE_URL env var; returns a status JSON, never raises | +| `db_query_value` | `db_query_value of sql` or `db_query_value of [sql, p1, p2]` | Execute query, return row 0 col 0 typed by its SQL type; `null` for SQL NULL, `""` for no rows | +| `db_execute` | `db_execute of sql` or `db_execute of [sql, p1, p2]` | Execute command with optional params; returns `"ok"` | +| `db_query_json` | `db_query_json of sql` or `db_query_json of [sql, p1, p2]` | Execute query, return all rows as a JSON array of objects, each value typed by its SQL type | + +### Failures raise (#888) + +`db_connect` is the only one that reports by return value — it hands back +`{"status": ...}` so a program can probe for a database without a `try`. +Every other db builtin **raises** a catchable `io` error when the statement +fails or there is no connection, carrying libpq's own first line +(`ERROR: relation "orders" does not exist`). A genuinely empty result is +still `[]` / `""`, and only that. + +They used to return `[]` / `""` for a syntax error, a missing table, a +revoked permission *and* an empty table alike, so a reporting script kept +printing "0 rows" forever after a schema change and a migration that did +nothing looked healthy in CI. + +```eigenscript +try: + rows is json_decode of (db_query_json of "SELECT * FROM orders") +catch e: + print of ("query failed: " + e.message) # e.kind is "io" +``` + +### SQL types survive the trip (#887) + +Values carry their column's SQL type rather than arriving as strings: + +| SQL type | Arrives as | Note | +|---|---|---| +| NULL (any column type) | `null` | Distinct from `""` — checked before the type | +| `boolean` | `true` / `false` → `1` / `0` | `if row.is_admin:` means what it reads as | +| `smallint`, `integer`, `bigint`, `oid` | number | `bigint` past 2^53 **raises** — see below | +| `real`, `double precision` | number | `NaN`/`Infinity` arrive as strings; JSON has no literal for them | +| `numeric` | **string** | Deliberate — see below | +| everything else | string | text, date, uuid, json, … unchanged | + +The mapping is a function of the column's SQL type alone, never of the +row's value: a column that decoded as a number for row 1 and a string for +row 100 would break `row.n + 1` on data rather than on schema. + +**`numeric` stays a string.** It is PostgreSQL's arbitrary-precision decimal +— the money type — and an EigenScript number is a binary double, which +cannot hold `numeric(38,10)` or even `0.1` exactly. Preserving the digits +is the safe default; `SELECT amount::float8` is the one-token opt-in to a +number when approximate is fine. Note `avg()` and `sum(numeric)` return +`numeric`, so those want the cast; `count(*)` and `sum(integer)` return +`bigint` and are already numbers. + +**A `bigint` past 2^53 raises** instead of silently rounding, naming the +column and the fix: + +``` +Error line 3: db: column 'id' value 9007199254740993 exceeds the exact-integer +range of a number (2^53); select it as text (id::text) to keep the digits +``` + +Before this, every value was a string: SQL `false` arrived as `"f"`, which +is a non-empty string and therefore **truthy**, so `if row.is_admin:` passed +for a non-admin; NULL and `''` were both `""`; and `9 > 10` was true because +`'9' > '1'`. ## Optional: Model Extension diff --git a/docs/DIAGNOSTICS.md b/docs/DIAGNOSTICS.md index a04bf3a9..c90cdf5e 100644 --- a/docs/DIAGNOSTICS.md +++ b/docs/DIAGNOSTICS.md @@ -72,7 +72,7 @@ below are the contract.) | `value` | right type, unacceptable value | `index must be an integer, got 1.5`, `chr of 0`, invalid channel | | `index_range` | index/slice outside bounds | `index 10 out of range (list length 3)` | | `parse` | runtime-surfaced parse/compile failure | `eval: parse error in code string`, `import: parse errors in 'm'` | -| `io` | the outside world failed | `import: cannot read 'm'`, `store_open: cannot create`, thread-create failure | +| `io` | the outside world failed | `import: cannot read 'm'`, `store_open: cannot create`, thread-create failure, `db: query failed: ERROR: relation "orders" does not exist` | | `limit` | engine resource cap hit | `call stack overflow`, `store_put: record too large`, route table full | | `sandbox` | sandbox policy denial or budget | `blocked in sandbox`, `sandbox memory budget exceeded` | | `interrupt` | host-requested abort (`eigs_abort`) | `aborted` | diff --git a/src/ext_db.c b/src/ext_db.c index 86b69830..032961b5 100644 --- a/src/ext_db.c +++ b/src/ext_db.c @@ -19,6 +19,10 @@ static int db_build_query(Value *arg, const char **sql, int *nparams, } if (!arg || arg->type != VAL_LIST || arg->data.list.count < 1 || arg->data.list.items[0]->type != VAL_STR) { + /* #888: the callers used to turn this into "[]"/"" — the same value a + * successful empty query returns. A malformed call is a program bug + * and must not look like a result. */ + rt_error(EK_TYPE, 0, "db: expected [sql, params...] with a string SQL first element"); return 0; } @@ -90,49 +94,249 @@ Value* builtin_db_connect(Value *arg) { return make_str("{\"status\": \"connected\", \"driver\": \"libpq\"}"); } +/* ---- #888: failures raise, they do not return a plausible success value ---- + * + * Every DB failure used to be laundered into the same value the success path + * returns for "no rows" ("[]" / "") — so a typo'd query, a dropped table, a + * revoked permission and a genuinely empty table were one indistinguishable + * outcome, and a reporting script kept printing "0 rows" forever after a + * schema change. LANGUAGE_CONTRACT.md:50-58 says programs never "report + * success on failure"; every other I/O surface in the runtime honors that. + * + * Kind is EK_IO ("the outside world failed") for both the connection and the + * statement. Not EK_VALUE: libpq reports a syntax error, a permission denial, + * a missing relation and a reset connection through one channel, so claiming + * the runtime knows the failure was a bad argument would be a guess. No new + * `db` kind either — DIAGNOSTICS.md's closed set classifies by the NATURE of + * the failure, not by which subsystem raised it. Callers discriminate on the + * message, which carries libpq's own text. */ +static void db_raise_stmt(PGresult *res, const char *what) { + const char *msg = NULL; + if (res) msg = PQresultErrorMessage(res); + if ((!msg || !msg[0]) && g_db_conn) msg = PQerrorMessage(g_db_conn); + if (!msg) msg = ""; + /* libpq messages are multi-line ("ERROR: ...\nLINE 1: ...\n ^\n"); the + * first line carries the diagnosis and a caught error's `message` must + * stay one line. */ + char buf[512]; + size_t n = 0; + while (msg[n] && msg[n] != '\n' && n + 1 < sizeof(buf)) { buf[n] = msg[n]; n++; } + while (n > 0 && (buf[n - 1] == ' ' || buf[n - 1] == '\r')) n--; + buf[n] = '\0'; + rt_error(EK_IO, 0, "db: %s failed: %s", what, n ? buf : "no message from server"); +} + +/* Returns 0 (already raised) when there is no usable connection. */ +static int db_require_conn(void) { + if (g_db_conn && PQstatus(g_db_conn) == CONNECTION_OK) return 1; + rt_error(EK_IO, 0, "db: not connected — call db_connect first%s", + g_db_conn ? " (connection is bad)" : ""); + return 0; +} + +/* ---- #887: SQL types survive the trip into EigenScript ---- + * + * Every column used to be emitted with eigs_json_escape_string, i.e. as a + * JSON string, because that is what libpq's text mode hands back. Three + * silent consequences: SQL false arrived as "f" and "f" is a non-empty + * string, so it is TRUTHY and `if row.is_admin:` passed for a non-admin; + * NULL arrived as "" and was indistinguishable from an empty string; and + * numbers compared lexicographically ('9' > '10'). + * + * PostgreSQL type OIDs are pinned by the wire protocol and stable across + * every server version, so they are spelled here rather than pulling in + * server-side catalog headers that libpq does not ship. */ +#define DB_OID_BOOL 16 +#define DB_OID_INT8 20 +#define DB_OID_INT2 21 +#define DB_OID_INT4 23 +#define DB_OID_OID 26 +#define DB_OID_FLOAT4 700 +#define DB_OID_FLOAT8 701 +#define DB_OID_NUMERIC 1700 + +/* The largest integer a double represents exactly. A bigint past this cannot + * round-trip through an EigenScript number. */ +#define DB_EXACT_INT_MAX 9007199254740992LL + +typedef enum { DBT_STR, DBT_NUM, DBT_BOOL } DbColType; + +/* ONE classifier, shared by db_query_json and db_query_value, so the two + * cannot drift on what a column's type is. + * + * The mapping is a function of the column's SQL type ALONE and never of the + * row's value: a column that decodes as a number for row 1 and a string for + * row 100 is a landmine (`row.n + 1` works until it doesn't), so "emit a + * number when this particular value happens to fit" is deliberately not what + * this does. + * + * NUMERIC stays a string on purpose. It is PostgreSQL's arbitrary-precision + * decimal — the money type — and converting it to a binary double is lossy + * by construction (0.1 is not a double; numeric(38,10) is not a double). + * Silently rounding money is the exact defect class this issue is about, so + * the digits are preserved and `SELECT amount::float8` is the one-token opt + * in to a number. int8 does NOT get that treatment: count(*), sum(int) and + * every serial key are int8, so leaving it a string would leave the filed + * bug unfixed for the majority of real numeric queries — instead it emits a + * number and RAISES on the values a double cannot represent (below). */ +static DbColType db_classify(unsigned int oid) { + switch (oid) { + case DB_OID_BOOL: return DBT_BOOL; + case DB_OID_INT2: + case DB_OID_INT4: + case DB_OID_OID: + case DB_OID_INT8: + case DB_OID_FLOAT4: + case DB_OID_FLOAT8: return DBT_NUM; + default: return DBT_STR; /* numeric, text, date, json, ... */ + } +} + +/* An exact-integer column whose value is past 2^53. Raising beats rounding: + * the value genuinely has no representation as an EigenScript number, and a + * silently rounded primary key is unrecoverable downstream. The message names + * the column and the fix. Returns 0 and raises; 1 when representable. */ +static int db_check_exact_int(unsigned int oid, const char *text, const char *colname) { + if (oid != DB_OID_INT8 && oid != DB_OID_INT4 && oid != DB_OID_INT2 && oid != DB_OID_OID) + return 1; + /* strtoLL, not strtod: converting to a double FIRST rounds 9007199254740993 + * to exactly 2^53, so a strtod-based comparison cannot see the one thing + * this check exists to detect. (It shipped that way to CI once — the live + * postgres job is what caught it.) An out-of-int64 literal saturates at + * LLONG_MAX, which is past the bound, so it still raises. */ + char *end = NULL; + long long v = strtoll(text, &end, 10); + if (end == text) return 1; /* not a plain integer literal; leave it */ + if (v > DB_EXACT_INT_MAX || v < -DB_EXACT_INT_MAX) { + rt_error(EK_VALUE, 0, + "db: column '%s' value %s exceeds the exact-integer range of a " + "number (2^53); select it as text (%s::text) to keep the digits", + colname ? colname : "?", text, colname ? colname : "col"); + return 0; + } + return 1; +} + +/* Emit one column value as JSON. Returns 0 when it raised. */ +static int db_emit_json_value(strbuf *sb, unsigned int oid, int isnull, + const char *text, const char *colname) { + if (isnull) { strbuf_append(sb, "null"); return 1; } + if (!text) text = ""; + switch (db_classify(oid)) { + case DBT_BOOL: + /* libpq text mode renders bool as exactly "t" or "f". */ + strbuf_append(sb, text[0] == 't' ? "true" : "false"); + return 1; + case DBT_NUM: + if (!db_check_exact_int(oid, text, colname)) return 0; + /* float8/float4 can be NaN / Infinity / -Infinity, which JSON has + * no literal for. Emitting them as `null` would collide with SQL + * NULL — the very confusion this issue removes — so they stay + * quoted: a caller doing arithmetic gets a loud type error rather + * than a value that is silently the wrong thing. */ + if (text[0] == 'N' || text[0] == 'I' || + (text[0] == '-' && text[1] == 'I')) { + eigs_json_escape_string(sb, text); + return 1; + } + strbuf_append(sb, text); /* libpq's decimal text is valid JSON */ + return 1; + case DBT_STR: + break; + } + eigs_json_escape_string(sb, text); + return 1; +} + /* ---- DB query builtins ---- */ Value* builtin_db_query_value(Value *arg) { - /* Run a SQL query and return the value from row 0 col 0 as a string. + /* Run a SQL query and return row 0 col 0, typed by its SQL type (#887). * Usage: db_query_value of "SELECT COUNT(*) FROM table" * or: db_query_value of ["SELECT name FROM t WHERE id=$1", id] - * Returns "" if no DB, query fails, or no rows. */ - if (!arg || (arg->type != VAL_STR && arg->type != VAL_LIST)) return make_str(""); - if (!g_db_conn || PQstatus(g_db_conn) != CONNECTION_OK) return make_str(""); + * Returns null for SQL NULL and "" for no rows; raises on failure (#888). */ + if (!arg || (arg->type != VAL_STR && arg->type != VAL_LIST)) { + rt_error(EK_TYPE, 0, "db_query_value expects a string or [sql, params...] (got %s)", + arg ? val_type_name(arg->type) : "null"); + return make_str(""); + } + if (!db_require_conn()) return make_str(""); PGresult *res = db_exec_from_arg(arg); - if (!res) return make_str(""); - if (PQresultStatus(res) != PGRES_TUPLES_OK || PQntuples(res) == 0) { + if (!res) return make_str(""); /* db_build_query already raised */ + if (PQresultStatus(res) != PGRES_TUPLES_OK) { + db_raise_stmt(res, "query"); PQclear(res); return make_str(""); } - const char *val = PQgetvalue(res, 0, 0); - Value *result = make_str(val ? val : ""); + if (PQntuples(res) == 0 || PQnfields(res) == 0) { PQclear(res); return make_str(""); } + /* NULL is not "" — the two used to be the same value here, so a caller + * could not tell "no value" from "empty value" for any column type. */ + if (PQgetisnull(res, 0, 0)) { PQclear(res); return make_null(); } + + unsigned int oid = (unsigned int)PQftype(res, 0); + const char *text = PQgetvalue(res, 0, 0); + const char *colname = PQfname(res, 0); + if (!text) text = ""; + Value *result = NULL; + switch (db_classify(oid)) { + case DBT_BOOL: + result = make_num(text[0] == 't' ? 1 : 0); + break; + case DBT_NUM: + if (!db_check_exact_int(oid, text, colname)) { PQclear(res); return make_str(""); } + /* Same rule as the JSON emitter: float4/float8 can be NaN / + * Infinity / -Infinity, and make_num's num_guard would turn + * those into 0 and 1e308 — a silently wrong number. Hand back + * the text so the caller sees what the column holds. */ + if (text[0] == 'N' || text[0] == 'I' || + (text[0] == '-' && text[1] == 'I')) { + result = make_str(text); + break; + } + result = make_num(strtod(text, NULL)); + break; + case DBT_STR: + result = make_str(text); + break; + } PQclear(res); return result; } Value* builtin_db_execute(Value *arg) { - /* Run a SQL command with optional params. Returns "ok" or "error". + /* Run a SQL command with optional params. Returns "ok"; raises on failure. * Usage: db_execute of "INSERT INTO ..." (no params) * or: db_execute of ["INSERT INTO t (a) VALUES ($1)", param1] */ - if (!g_db_conn || PQstatus(g_db_conn) != CONNECTION_OK) return make_str("no_db"); + if (!arg || (arg->type != VAL_STR && arg->type != VAL_LIST)) { + rt_error(EK_TYPE, 0, "db_execute expects a string or [sql, params...] (got %s)", + arg ? val_type_name(arg->type) : "null"); + return make_str("error"); + } + if (!db_require_conn()) return make_str("error"); PGresult *res = db_exec_from_arg(arg); - if (!res) return make_str("error"); + if (!res) return make_str("error"); /* db_build_query already raised */ int ok = (PQresultStatus(res) == PGRES_COMMAND_OK || PQresultStatus(res) == PGRES_TUPLES_OK); + if (!ok) db_raise_stmt(res, "execute"); PQclear(res); return make_str(ok ? "ok" : "error"); } Value* builtin_db_query_json(Value *arg) { - /* Run SQL and return all rows as a JSON array of objects. + /* Run SQL and return all rows as a JSON array of objects, each value + * carrying its SQL type (#887). * Usage: db_query_json of "SELECT id, name FROM table" * or: db_query_json of ["SELECT id, name FROM table WHERE id=$1", id] - * Returns "[]" if no DB, error, or no rows. */ - if (!arg || (arg->type != VAL_STR && arg->type != VAL_LIST)) return make_str("[]"); - if (!g_db_conn || PQstatus(g_db_conn) != CONNECTION_OK) return make_str("[]"); + * Returns "[]" for a genuinely empty result; raises on failure (#888). */ + if (!arg || (arg->type != VAL_STR && arg->type != VAL_LIST)) { + rt_error(EK_TYPE, 0, "db_query_json expects a string or [sql, params...] (got %s)", + arg ? val_type_name(arg->type) : "null"); + return make_str("[]"); + } + if (!db_require_conn()) return make_str("[]"); PGresult *res = db_exec_from_arg(arg); - if (!res) return make_str("[]"); + if (!res) return make_str("[]"); /* db_build_query already raised */ if (PQresultStatus(res) != PGRES_TUPLES_OK) { + db_raise_stmt(res, "query"); PQclear(res); return make_str("[]"); } @@ -150,7 +354,13 @@ Value* builtin_db_query_json(Value *arg) { if (c > 0) strbuf_append(&sb, ", "); eigs_json_escape_string(&sb, PQfname(res, c)); strbuf_append(&sb, ": "); - eigs_json_escape_string(&sb, PQgetvalue(res, r, c)); + if (!db_emit_json_value(&sb, (unsigned int)PQftype(res, c), + PQgetisnull(res, r, c), PQgetvalue(res, r, c), + PQfname(res, c))) { + strbuf_free(&sb); + PQclear(res); + return make_str("[]"); + } } strbuf_append_char(&sb, '}'); } diff --git a/tests/test_db.eigs b/tests/test_db.eigs index 3dab7f83..254e9819 100644 --- a/tests/test_db.eigs +++ b/tests/test_db.eigs @@ -1,8 +1,14 @@ # Database extension tests. # Runs only when the binary is compiled with EIGENSCRIPT_EXT_DB=1 -# (gated by run_all_tests.sh). Tests exercise the "no-DATABASE_URL" / -# "no-connection" safety paths. Real-DB tests are a TODO and should be -# added behind an env-var gate when a Postgres instance is available. +# (gated by run_all_tests.sh). +# +# Two halves: +# * the no-connection half runs EVERYWHERE the db extension is compiled in, +# and covers #888 — a DB failure raises instead of returning the same +# value a successful empty query returns; +# * the live half (DB09+) needs a real server via DATABASE_URL — the CI +# postgres service container provides one — and covers #887, the SQL +# type -> EigenScript value mapping. # ---- db_connect: without DATABASE_URL returns a well-formed status JSON. # NOTE: Some CI environments DO set DATABASE_URL, so we accept either the @@ -11,65 +17,112 @@ conn is db_connect of null has_status is (contains of [conn, "status"]) assert of [has_status == 1, "DB01 db_connect returns JSON containing 'status'"] -# ---- db_query_value: without a live connection returns "" -# (If a connection happens to be live, this still succeeds since COUNT(*) of -# nonexistent table returns "" on error too.) -v is db_query_value of "SELECT 1 FROM _eigs_no_such_table_XXXXX" -assert of [v == "", "DB02 db_query_value on bad table returns empty string"] +live is (contains of [conn, "connected"]) == 1 -# ---- db_query_value: non-string arg returns "" -v_bad is db_query_value of 42 -assert of [v_bad == "", "DB03 db_query_value non-string returns empty string"] - -# ---- db_query_json: bad/empty query returns "[]" -j is db_query_json of "SELECT 1 FROM _eigs_no_such_table_XXXXX" -assert of [j == "[]", "DB04 db_query_json on bad table returns '[]'"] +# ---- #888: a query without a connection RAISES. It used to return "", +# which is also what a successful query over an empty table returns. +if not live: + c02 is 0 + m02 is "" + k02 is "" + try: + db_query_value of "SELECT 1" + catch e: + c02 is 1 + m02 is e.message + k02 is e.kind + assert of [c02 == 1, "DB02 db_query_value without a connection raises"] + assert of [(contains of [m02, "not connected"]) == 1, "DB02 message names the cause"] + assert of [k02 == "io", "DB02 kind is io"] -j_bad is db_query_json of 42 -assert of [j_bad == "[]", "DB05 db_query_json non-string returns '[]'"] + c04 is 0 + m04 is "" + try: + db_query_json of "SELECT 1" + catch e: + c04 is 1 + m04 is e.message + assert of [c04 == 1, "DB04 db_query_json without a connection raises"] + assert of [(contains of [m04, "not connected"]) == 1, "DB04 message names the cause"] -# ---- Parameterized SELECT forms. -# Without a live DB these should keep the same safe empty result. With a live -# DB they prove SELECT helpers route params through PQexecParams. -v_param is db_query_value of ["SELECT $1::text", "safe-param"] -if (contains of [conn, "connected"]) == 1: - assert of [v_param == "safe-param", "DB06 db_query_value supports params"] + c08 is 0 + try: + db_execute of "SELECT 1" + catch e: + c08 is 1 + assert of [c08 == 1, "DB08 db_execute without a connection raises"] else: - assert of [v_param == "", "DB06 db_query_value param form is safe without DB"] + print of "no-connection checks skipped (a live DATABASE_URL is set)" + +# ---- A malformed call is a program bug and must not look like a result. +# These raise whether or not a connection exists (the arg never reaches SQL). +c03 is 0 +m03 is "" +k03 is "" +try: + db_query_value of 42 +catch e: + c03 is 1 + m03 is e.message + k03 is e.kind +assert of [c03 == 1, "DB03 db_query_value on a non-string arg raises"] +assert of [k03 == "type_mismatch", "DB03 kind is type_mismatch"] -j_param is db_query_json of ["SELECT $1::text AS value", "safe-json"] -if (contains of [conn, "connected"]) == 1: +c05 is 0 +try: + db_query_json of 42 +catch e: + c05 is 1 +assert of [c05 == 1, "DB05 db_query_json on a non-string arg raises"] + +c05b is 0 +try: + db_execute of null +catch e: + c05b is 1 +assert of [c05b == 1, "DB05b db_execute on a null arg raises"] + +# ---- Parameterized SELECT forms route params through PQexecParams. +if live: + v_param is db_query_value of ["SELECT $1::text", "safe-param"] + assert of [v_param == "safe-param", "DB06 db_query_value supports params"] + j_param is db_query_json of ["SELECT $1::text AS value", "safe-json"] assert of [(contains of [j_param, "safe-json"]) == 1, "DB07 db_query_json supports params"] -else: - assert of [j_param == "[]", "DB07 db_query_json param form is safe without DB"] - -# ---- db_execute: null/bad arg returns 'error' or 'no_db' -# (Depends on whether a connection happens to be active in the env.) -e is db_execute of null -is_safe is ((e == "error") or (e == "no_db")) -assert of [is_safe == 1, "DB08 db_execute null returns 'error' or 'no_db'"] - -# ---- Live-DB table round-trip (runs only with a real connection — the -# CI postgres service container provides one via DATABASE_URL). Covers -# db_execute's COMMAND_OK path, parameterized INSERT, multi-row -# db_query_json assembly, and db_query_value over a real table. -if (contains of [conn, "connected"]) == 1: + +# ---- Live-DB table round-trip. Covers db_execute's COMMAND_OK path, +# parameterized INSERT, multi-row db_query_json assembly, db_query_value +# over a real table, and the #887 type mapping. +if live: db_execute of "DROP TABLE IF EXISTS eigs_ci_smoke" c1 is db_execute of "CREATE TABLE eigs_ci_smoke (id INT, name TEXT)" assert of [c1 == "ok", "DB09 CREATE TABLE returns ok"] i1 is db_execute of ["INSERT INTO eigs_ci_smoke VALUES ($1, $2)", 1, "alpha"] assert of [i1 == "ok", "DB10 parameterized INSERT returns ok"] db_execute of ["INSERT INTO eigs_ci_smoke VALUES ($1, $2)", 2, "beta"] + # COUNT(*) is int8 -> a real number now, not the string "2". n is db_query_value of "SELECT COUNT(*) FROM eigs_ci_smoke" - assert of [n == "2", "DB11 COUNT over real table"] + assert of [n == 2, "DB11 COUNT over real table is a number"] + assert of [(type of n) == "num", "DB11 COUNT is typed num"] one is db_query_value of ["SELECT name FROM eigs_ci_smoke WHERE id = $1", 2] assert of [one == "beta", "DB12 parameterized SELECT value"] rows is db_query_json of "SELECT id, name FROM eigs_ci_smoke ORDER BY id" decoded is json_decode of rows assert of [(len of decoded) == 2, "DB13 query_json returns both rows"] assert of [decoded[0].name == "alpha", "DB14 query_json field value"] - bad is db_execute of "THIS IS NOT SQL" - assert of [bad == "error", "DB15 malformed SQL returns error"] + + # ---- #888: a malformed statement raises and carries libpq's text ---- + c15 is 0 + m15 is "" + k15 is "" + try: + db_execute of "THIS IS NOT SQL" + catch e: + c15 is 1 + m15 is e.message + k15 is e.kind + assert of [c15 == 1, "DB15 malformed SQL raises"] + assert of [k15 == "io", "DB15 kind is io"] + assert of [(contains of [m15, "syntax error"]) == 1, "DB15 message carries libpq's text"] # ---- #356: param validation raises instead of silently proceeding ---- q17 is ["SELECT $1", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17] @@ -94,6 +147,92 @@ if (contains of [conn, "connected"]) == 1: assert of [c17a == 1, "DB17 list param raises"] assert of [(contains of [m17a.message, "not a string or number"]) == 1, "DB17 message names the type rule"] + # ---- #888: a failed query is distinguishable from an empty result ---- + empty is db_query_json of "SELECT id FROM eigs_ci_smoke WHERE id = 999" + assert of [empty == "[]", "DB18 a genuinely empty result set is still []"] + + c19 is 0 + m19 is "" + try: + db_query_json of "SELECT * FROM _eigs_no_such_table_XXXXX" + catch e: + c19 is 1 + m19 is e.message + assert of [c19 == 1, "DB19 a missing table raises rather than returning []"] + assert of [(contains of [m19, "does not exist"]) == 1, "DB19 message names the missing relation"] + + c20 is 0 + try: + db_query_json of "SELECT nonsense syntax FROM FROM" + catch e: + c20 is 1 + assert of [c20 == 1, "DB20 a syntax error raises rather than returning []"] + + # ---- #887: SQL types survive the trip ---- + t is db_query_json of "SELECT true AS yes, false AS no, null::boolean AS unknown, 0 AS zero, null::text AS ntext, '' AS empty, 42 AS n" + d is (json_decode of t)[0] + + # 1. false is FALSY. It used to arrive as "f" — a non-empty string, and + # every non-empty string is truthy, so `if row.flag:` was inverted. + assert of [d.yes == 1, "DB21 SQL true decodes to 1"] + assert of [d.no == 0, "DB21 SQL false decodes to 0"] + took is 0 + if d.no: + took is 1 + assert of [took == 0, "DB21 SQL false is falsy in an if"] + zero_took is 0 + if d.zero: + zero_took is 1 + assert of [zero_took == 0, "DB21 SQL 0 is falsy in an if"] + + # 2. NULL is distinguishable from the empty string. + assert of [(type of d.unknown) == "none", "DB22 SQL NULL decodes to null"] + assert of [(type of d.ntext) == "none", "DB22 a NULL text column decodes to null"] + assert of [d.empty == "", "DB22 an empty string stays an empty string"] + assert of [(type of d.empty) == "str", "DB22 an empty string is still a str"] + # The claim in its plainest form: the two used to be one value. + assert of [(d.ntext == d.empty) == 0, "DB22 a NULL text column is NOT the empty string"] + assert of [(d.ntext == null) == 1, "DB22 a NULL column compares equal to null"] + + # 3. Numbers are numbers — ordering and arithmetic, not text. + assert of [(type of d.n) == "num", "DB23 an int column decodes to num"] + assert of [(d.n + 1) == 43, "DB23 arithmetic works on a numeric column"] + ord_rows is json_decode of (db_query_json of "SELECT 9 AS a, 10 AS b") + assert of [ord_rows[0].a < ord_rows[0].b, "DB23 numbers order numerically, not lexicographically"] + + # db_query_value applies the SAME mapping (one classifier, no drift). + assert of [(db_query_value of "SELECT false") == 0, "DB24 db_query_value types booleans"] + assert of [(type of (db_query_value of "SELECT null::int")) == "none", "DB24 db_query_value returns null for SQL NULL"] + assert of [(type of (db_query_value of "SELECT ''")) == "str", "DB24 db_query_value keeps '' a str"] + assert of [(db_query_value of "SELECT 7::float8") == 7, "DB24 db_query_value types floats"] + # NaN would become 0 through make_num's num_guard — a silently wrong + # number — so it comes back as text on both paths. + assert of [(db_query_value of "SELECT 'NaN'::float8") == "NaN", "DB24 NaN stays text, not 0"] + nan_row is (json_decode of (db_query_json of "SELECT 'NaN'::float8 AS x"))[0] + assert of [nan_row.x == "NaN", "DB24 NaN stays text in JSON too"] + + # numeric stays a STRING on purpose: it is arbitrary-precision decimal and + # a binary double cannot hold it. ::float8 is the documented opt-in. + num_row is (json_decode of (db_query_json of "SELECT 1.10::numeric AS m, 1.10::float8 AS f"))[0] + assert of [(type of num_row.m) == "str", "DB25 numeric stays a string (exact digits preserved)"] + assert of [num_row.m == "1.10", "DB25 numeric keeps its scale"] + assert of [(type of num_row.f) == "num", "DB25 ::float8 is the opt-in to a number"] + + # An exact-integer column past 2^53 RAISES rather than silently rounding. + c26 is 0 + m26 is "" + try: + db_query_json of "SELECT 9007199254740993::bigint AS big" + catch e: + c26 is 1 + m26 is e.message + assert of [c26 == 1, "DB26 a bigint past 2^53 raises instead of rounding"] + assert of [(contains of [m26, "big"]) == 1, "DB26 message names the column"] + assert of [(contains of [m26, "::text"]) == 1, "DB26 message names the fix"] + # ...and the boundary itself is fine. + ok53 is (json_decode of (db_query_json of "SELECT 9007199254740992::bigint AS big"))[0] + assert of [ok53.big == 9007199254740992, "DB26 2^53 itself is representable"] + db_execute of "DROP TABLE eigs_ci_smoke" else: print of "live-DB checks skipped (no connection)"