commit 608ef4f (c1/copilot/87b2968-defect-tests) Author: Noah Misch AuthorDate: Fri Jul 31 22:48:42 2026 +0000 Commit: Noah Misch CommitDate: Fri Jul 31 22:48:42 2026 +0000 Add TAP test for builtin-provider identifier folding split Commit 87b2968df0f866aaccb6ba69adf284e3c4a79454 routed backend unquoted-identifier case folding through the database locale provider's method table and set the builtin provider's .downcase_ident to NULL, i.e. ASCII-only folding. It did not update the frontend copy of that logic in src/port/pgstrcasecmp.c / patternToSQLRegex(), even though pgstrcasecmp.c states "NB: this code should match downcase_truncate_identifier() in scansup.c." In a single-byte (LATIN1) database using the builtin provider whose datctype matches the client's LC_CTYPE, the two sides agreed before the commit but can now disagree: the backend keeps byte 0xC4 distinct from 0xE4, while an ISO-8859-1 client still folds 0xC4 to 0xE4. As a result psql's \d resolves a different, nonexistent relation than identical unquoted SQL sent to the same server. The new test asserts the fix-direction-agnostic invariant that if the backend resolves an unquoted name (to_regclass() is non-null), then psql's \d on the same unquoted spelling must show that relation. It fails on unmodified master and passes once the frontend folding is made provider-aware or ASCII-only. The divergence requires a single-byte ISO-8859-1 locale on the client, so the test skips where none is installed (and on Windows). An accompanying report, src/bin/psql/DEFECTS_87b2968.md, documents related untested variants (\password, tab completion, ECPG) and the rejected candidates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/bin/psql/DEFECTS_87b2968.md | 465 ++++++++++++++++++++++ src/bin/psql/meson.build | 1 + src/bin/psql/t/040_builtin_high_bit_identifier.pl | 146 +++++++ 3 files changed, 612 insertions(+) diff --git a/src/bin/psql/DEFECTS_87b2968.md b/src/bin/psql/DEFECTS_87b2968.md new file mode 100644 index 0000000..b86ea7a --- /dev/null +++ b/src/bin/psql/DEFECTS_87b2968.md @@ -0,0 +1,465 @@ +# User-visible defects from commit 87b2968 still present in master + +Commit **87b2968df0f866aaccb6ba69adf284e3c4a79454** ("downcase_identifier(): use +method table from locale provider.", Jeff Davis, 2025-12-16; parent +`7f007e4a044a713df5320fca09621d6ba8e632ba`) routed backend unquoted-identifier +case folding through the database locale provider's method table +(`downcase_truncate_identifier()` → `pg_downcase_ident()` → +`default_locale->ctype->downcase_ident`) and set the **builtin** provider's +`.downcase_ident` to `NULL`, i.e. ASCII-only folding. It did **not** update the +frontend copies of that logic, whose canonical source +(`src/port/pgstrcasecmp.c`) carries the explicit contract comment: + +> `NB: this code should match downcase_truncate_identifier() in scansup.c.` + +This report is produced by an automated defect-hunting workflow. It accompanies +the TAP test `src/bin/psql/t/040_builtin_high_bit_identifier.pl`, which encodes +the *correct* (post-fix) behavior and therefore **fails on unmodified master** +(1 of 3 assertions), demonstrating the tested defect. The test is locale-gated: +it `skip_all`s unless a single-byte ISO-8859-1 client locale is installed. + +The verdict is **not** that the refactoring is wrong. For the builtin/libc +providers, 87b2968 made unquoted-identifier folding *consistent with* `lower()` +within a database (a correctness improvement) and pg_dump round-trips high-bit +names faithfully. The one still-present, user-visible defect is that the commit +changed one side of a documented two-sided contract and left the **frontend** +side (psql / libpq / ECPG) unchanged, so a client whose locale matches the +server `datctype` — which *agreed* before the commit — now diverges for the +builtin provider. + +--- + +## The verbatim user prompt that drove this workflow + +``` +Make a large workflow, with at most 90 agents, to write test cases covering +user-visible defects in these commits: + +f81bf78ce12b9fd3e50eb00dd875440007262ec4 +147602822597204aa436415ebe295926b268ab5c +19b966243c38196a33b033fb0c259dcf760c0d69 +630706ced04e3a7a7f0070f4e8fb88f7503a1016 +87b2968df0f866aaccb6ba69adf284e3c4a79454 +0a90df58cf38cf68d59c6841513be98aeeff250e +8185bb53476378443240d57f7d844347d5fae1bf +dbf217c1c7c2744a18db489c255255e07cfbb110 + +Use your own worktrees; disregard the present dir except as repository to +which to attach your worktree. The workflow should first look for extant +user-visible defects (still present in master). If it finds any, write a test +case covering some of those defects. If any defects found weren't suitable to +test, describe them in a report. Include in the report the prompt I used, the +latest hash of the branches you consulted, model version, etc. Commit the +tests and the report (if any) on a fresh branch per commit under review. +``` + +--- + +## Provenance + +* **Repository:** PostgreSQL, git dir `/home/nm/src/pg/postgresql`. +* **Branch consulted:** `master` (== `origin/master`) at + **`481052c7754013d44a89f50912ba6845398f88df`** ("On Windows, make link(2) + report ENOTSUP when appropriate.", CommitDate 2026-07-31 14:39:37 -0400). + **No other branch was consulted.** All eight worktrees were created from this + commit. +* **Commit under review:** `87b2968df0f866aaccb6ba69adf284e3c4a79454` + — "downcase_identifier(): use method table from locale provider." (Jeff Davis). + **Parent:** `7f007e4a044a713df5320fca09621d6ba8e632ba`. +* **Agent / model:** GitHub Copilot CLI **v1.0.77**; orchestrating model + **Claude Opus 5** (`claude-opus-5`). The hunt deliberately used six different + models for finding diversity — `claude-opus-4.8`, `gpt-5.5`, + `claude-sonnet-4.6`, `gemini-3.1-pro-preview`, `gpt-5.6-sol`, + `claude-opus-4.6` — with triage on `claude-opus-4.8`. +* **Host / toolchain:** Debian; gcc (Debian 14.2.0-19) 14.2.0; ICU 76.1; + meson 1.7.0 / ninja 1.12.1; `postgres (PostgreSQL) 20devel`. +* **Build configuration** (every worktree): + `meson setup build --prefix=/inst -Dcassert=true + -Dtap_tests=enabled -Dicu=enabled -Dssl=openssl -Dbuildtype=debug`. +* **OS locales generated for the hunt** (`locale -a`): + `C C.utf8 POSIX de_DE.iso88591 de_DE.utf8 el_GR.utf8 en_US.iso88591 + en_US.utf8 fr_FR.utf8 ja_JP.eucjp ja_JP.utf8 lt_LT.utf8 ru_RU.koi8r + tr_TR.iso88599 tr_TR.utf8`. +* **Workflow shape:** a four-wave, ~70-agent workflow across the eight commits. + Wave 1 ran five-to-six independent *hunter* agents per commit (each on one of + the six models above, from a distinct analysis angle: diff-semantics, + initialization-order/timing, empirical case-folding, provider×encoding matrix, + frontend identifier users, and robustness) against a prebuilt server. Wave 2 + ran one *triage* agent per commit to reproduce, deduplicate, and classify the + candidates (this commit's triage additionally re-evaluated its own initial + verdict after a challenge, promoting the frontend contract-break to testable). + Wave 3 (this agent) writes the test and report and commits them on a fresh + per-commit branch. Wave 4 verifies the deliverables. + +--- + +## Defects covered by the accompanying test + +### T1 — psql `\d` resolves an unquoted high-bit identifier to a different object than the builtin-provider backend + +* **File:line (master):** + * backend, builtin folding is ASCII-only: + `src/backend/utils/adt/pg_locale_builtin.c:266` (`.downcase_ident = NULL`) + → `src/backend/utils/adt/pg_locale.c:1369` `pg_downcase_ident()` takes the + `locale->ctype->downcase_ident == NULL` branch → `strlower_c()` (ASCII). + * backend entry point: `src/backend/parser/scansup.c:38` + `downcase_truncate_identifier()` → `:47,:60` `downcase_identifier()` / + `pg_downcase_ident(result, len + 1, ident, len)`. + * frontend still folds high-bit bytes per the **client** libc locale: + `src/fe_utils/string_utils.c:1282-1288` `patternToSQLRegex()` + (`else if (!inquotes && isupper((unsigned char) ch)) … pg_tolower(...)`), + which underlies psql `\d`; the shared helper is + `src/port/pgstrcasecmp.c` `pg_tolower()`/`pg_strcasecmp()`. + * the broken contract is stated at **`src/port/pgstrcasecmp.c:14`**: + *"NB: this code should match downcase_truncate_identifier() in scansup.c."* + +* **Root cause.** Before the commit, `downcase_identifier()` was + **provider-agnostic** and folded high-bit single-byte bytes with the + process-global `isupper()`/`tolower()`. From + `git show 87b2968^:src/backend/parser/scansup.c`: + + ```c + for (i = 0; i < len; i++) + { + unsigned char ch = (unsigned char) ident[i]; + + if (ch >= 'A' && ch <= 'Z') + ch += 'a' - 'A'; + else if (enc_is_single_byte && IS_HIGHBIT_SET(ch) && isupper(ch)) + ch = tolower(ch); + result[i] = (char) ch; + } + ``` + + `CheckMyDatabase()`/`init_database_collation()` pins the process global + `LC_CTYPE` to the database `datctype` (`src/backend/utils/init/postinit.c`) + before any SQL is lexed, so in a **builtin**-provider LATIN1 database with + `datctype = de_DE.iso88591` the *old* backend folded `0xC4` ("Ä") → `0xE4` + ("ä"). A psql running under a matching `de_DE.iso88591` `LC_CTYPE` folds the + same way, so the two **agreed**, and `Ä_t`/`ä_t` could not coexist as distinct + unquoted identifiers. 87b2968 set builtin's `.downcase_ident = NULL` + (ASCII-only), so the backend now keeps `0xC4` and `0xE4` distinct while the + frontend still folds `0xC4`→`0xE4`. + +* **Attributable to 87b2968 (criterion 3 — "the commit was supposed to handle + this case and didn't").** The frontend folder is the *other half* of the + contract that `src/port/pgstrcasecmp.c:14` explicitly says must match + `downcase_truncate_identifier()`. 87b2968 changed + `downcase_truncate_identifier()`'s result for the builtin provider and did not + touch the frontend half, newly breaking agreement for a client whose locale + matches the server `datctype`. `git log 87b2968..master -- + src/fe_utils/string_utils.c src/bin/psql/psqlscanslash.l src/port/pgstrcasecmp.c` + shows no follow-up change to this folding. + +* **User-visible symptom.** In a single-byte (LATIN1) builtin-provider + database, a table created as `"Ä_t"` (byte `0xC4`) is addressable unquoted as + `Ä_t` (the backend folds ASCII-only). A psql under an 8-bit `LC_CTYPE` folds + `0xC4`→`0xE4` locally, so `\d Ä_t` fails to find it — and if an `ä_t` table + also exists, `\d`, tab completion, and `\password` operate on the *wrong* + object. The client acts on a different object than identical unquoted SQL sent + to the same server. + +* **How the test detects it.** The test asserts the fix-direction-agnostic + invariant *"if `to_regclass(unquoted)` is non-null then psql `\d` on the same + unquoted spelling must show that relation."* It: + 1. inits a cluster with + `--locale-provider=builtin --builtin-locale=C --lc-ctype=$loc8 + --lc-collate=$loc8 --encoding=LATIN1` (with `$loc8` an installed ISO-8859-1 + locale, so the client locale equals the server `datctype`); + 2. `CREATE TABLE "chr(0xC4)_t"(umark int)`; + 3. asserts `SELECT to_regclass('…') IS NOT NULL` is `t` (**passes** — the + backend resolves the unquoted name, proving the object exists and the later + failure is about folding, not a missing object/locale/registration); + 4. control: under `LC_CTYPE=C` (no high-bit folding) `\d` finds the table + (**passes**); + 5. under `LC_CTYPE=$loc8`, asserts `\d` stdout contains the column `umark`. + On master this **fails** — psql folds `0xC4`→`0xE4`, so `\d` resolves a name + the backend never created. Observed failure: + + ``` + # Failed test 'psql \d resolves the same relation the backend does (client LC_CTYPE=de_DE.iso88591)' + # at t/040_builtin_high_bit_identifier.pl line 119. + # '' + # doesn't match '(?^:umark)' + # Frontend/backend identifier folding disagree. + # backend to_regclass(unquoted high-bit name) = 't' (relation exists) + # psql \d rc = 0 + # psql \d stdout = (empty) + # psql \d stderr = (empty) + # On master the builtin backend keeps byte 0xC4 while an ISO-8859-1 psql folds 0xC4 to 0xE4, so \d looks up the wrong (nonexistent) name. + # Looks like you failed 1 test of 3. + ``` + + Any legitimate fix satisfies the invariant in either direction: making the + frontend folding ASCII-only / provider-aware (the realistic fix, matching the + commit series' trajectory), or reverting the backend. The test therefore does + **not** bake in the buggy output. + +* **The natural fix** is to make the frontend folding provider-aware (or + ASCII-only) so it once again matches `downcase_truncate_identifier()`, restoring + the `src/port/pgstrcasecmp.c:14` contract. + +* **Skip caveat (honest limitation).** The divergence requires an ISO-8859-1 + locale **on the client**, where `isupper(0xC4)` is true. Under `C`, `C.UTF-8`, + or any UTF-8 client locale the frontend does not fold `0xC4` and there is no + divergence (verified). So the test `skip_all`s where no single-byte + ISO-8859-1 locale is installed (and on Windows). There is **no** + always-available-locale variant and **no** backend-only manifestation (see + "Candidates examined and rejected"), so this skip is unavoidable. + +--- + +## Defects found but NOT covered by a test + +All of these share T1's root cause (a frontend/preprocessor case-folder that +still uses the client process locale for high-bit bytes) and its locale gate +(they need an installed ISO-8859-1 client locale). They were reproduced by the +Wave-1 frontend hunter with pasted output. + +### R2 — psql `\password` changes the password of the wrong role + +* **File:line:** `src/bin/psql/psqlscanslash.l:743-766` + (`dequote_downcase_identifier()` folds via `pg_tolower()`); its own comment at + `:734` warns the result *"could differ if LC_CTYPE is different in the + frontend."* `\password` reads its role argument through this path + (`OT_SQLID`). +* **Symptom:** With roles `Ä_role` (`0xC4`) and `ä_role` (`0xE4`) both present on + a builtin-provider server, `\password Ä_role` under `LC_CTYPE=de_DE.iso88591` + prompts for and sets the password of **`ä_role`** — a silent + wrong-credential-target, more dangerous than `\d` because it mutates state. +* **Why not tested:** `\password` reads the new password from a TTY, so a + deterministic test needs a pseudo-terminal (IPC::Run `interactive`/`pty`), + which is flaky and platform-sensitive; the `\d` path exercises the identical + folding bug without a PTY. + +### R3 — psql tab completion completes against the wrong object + +* **File:line:** `src/bin/psql/tab-complete.in.c:6816` `parse_identifier()`, + folding high-bit bytes at `:6890-6891` + (`else if (enc_is_single_byte && IS_HIGHBIT_SET(ch) && isupper(ch)) ch = tolower(ch);`) + — a literal copy of the *pre-87b2968* backend loop, now out of sync with the + builtin backend. +* **Symptom:** typing `ALTER TABLE Ä_target DROP COLUMN ` offers a column + from the `ä_target` table, producing SQL that errors against `Ä_target`. +* **Why not tested:** readline tab-completion tests require a PTY and readline, + are inherently timing-sensitive, and are gated on `$ENV{with_readline}`. + +### R4 — ECPG preprocessor folds cursor / prepared-statement names differently from the backend + +* **File:line:** `src/interfaces/ecpg/preproc/ecpg.trailer:358` + (`str[i+1] = tolower((unsigned char) …)` on unquoted `prepared_name`), and + `:412`/`:430` (cursor-name comparison via `pg_strcasecmp()` → + `src/port/pgstrcasecmp.c`, raising `cursor "%s" is already defined`). +* **Symptom:** run under `LC_CTYPE=de_DE.iso88591` against a builtin LATIN1 + backend, ECPG (a) rejects the two backend-valid distinct cursor names `0xC4` + and `0xE4` as a duplicate (`ERROR … is already defined`, exit 3), and (b) folds + an unquoted prepared name `0xC4`→`0xE4`, so a later raw `DEALLOCATE` with the + source spelling fails with `prepared statement "…" does not exist`. +* **Why not tested (also triage R2):** the ECPG preprocessor's folding is + longstanding and *untouched* by 87b2968; attribution is "exposed", not + "introduced". A test would characterize preprocessor behavior, and the harness + must invoke `ecpg` under a specific 8-bit `LC_CTYPE`, compile, link, and run a + generated C program — far heavier than the psql `\d` path for the same + underlying bug. + +--- + +## Candidates examined and rejected + +The hunt surfaced two candidates that a Wave-1 hunter labeled "confirmed". +**Both were rejected on re-verification against master `481052c`.** + +### Rejected — "the builtin provider stopped folding high-bit identifiers per `datctype`" (regression framing) + +Real behavior change, but **intended and more correct** (criterion 1: not a +defect). In a LATIN1 database, comparing unquoted-identifier folding +(`parse_ident`) against `lower()` under the database default collation, +byte-for-byte: + +``` +provider=libc (de_DE.iso88591): ident 61c462 -> 61e462 lower 61c462 -> 61e462 AGREE +provider=builtin (C, datctype de_DE): ident 61c462 -> 61c462 lower 61c462 -> 61c462 AGREE +``` + +The brief's primary consistency oracle ("unquoted folding vs `lower()` under the +database default collation") **passes on master for both providers**. Crucially, +*before* 87b2968 the builtin database was internally **inconsistent** (identifier +folding used the global `LC_CTYPE`=`datctype` while `lower()` was builtin-C +ASCII); the commit makes them agree. Different providers folding differently is +by design; the change was reviewed by two core committers (Peter Eisentraut, +Chao Li). The only residual fallout is a narrow pg_upgrade cross-version +spelling change (a name folded on a pre-PG20 builtin-C single-byte cluster is +reachable only via its lowercase/quoted spelling after upgrade) — not in-tree +testable and arguably acceptable. The genuine, still-present consequence is the +*frontend* half that was not updated to match — that is T1. + +### Rejected — "ICU single-byte split-brain identifier folding tied to the postmaster environment" + +The stated **mechanism is false on master** (criteria 2 & 3). Sibling commit +`0a90df5` ("Avoid global LC_CTYPE dependency in pg_locale_icu.c", +`87b2968..master`) rewrote `downcase_ident_icu()` to use +`isupper_l()`/`tolower_l()` bound to a `locale_t` built from the database +`datctype`, not the environment. Restarting the *same* ICU LATIN1 datadir under +two postmaster environments yields **byte-identical** identifier folding: + +``` +ICU datctype=C, LATIN1: + postmaster LC_ALL=de_DE.iso88591: ident A Ä B -> 61c462 + postmaster LC_ALL=C (same datadir): ident A Ä B -> 61c462 +``` + +Identifier folding is invariant across the postmaster's `LC_ALL`, so it is bound +to `datctype`, not the environment. The residual ICU-vs-`lower()` mismatch for a +`datctype=C` ICU database is **pre-existing** (the pre-87b2968 code, using the +global `LC_CTYPE` pinned to `datctype=C`, produced the identical bytes) and +intended ("historical libc compatibility for single-byte encodings"). Not +attributable to 87b2968. + +### Rejected — pg_dump emits unrestorable output for high-bit unquoted names + +**Disproven; pg_dump is safe.** `fmtIdEnc()`/`quote_identifier()` treat only +ASCII `[a-z_][a-z0-9_]*` as safe and **quote every high-bit byte**. A full +dump → restore round-trip in a builtin LATIN1 database containing **both** +`Ä_t` (`0xC4`) and `ä_t` (`0xE4`), comparing relation-name byte sets, was +reproduced firsthand for this report: + +``` +source relnames (hex): c45f74 e45f74 +pg_dump emits: CREATE TABLE public."<0xC4>_t" ( … / CREATE TABLE public."<0xE4>_t" ( … +restored relnames (hex): c45f74 e45f74 -- identical; both preserved +``` + +Faithful. There is thus **no backend-only manifestation** of T1: the frontend +is the only inconsistent side. + +### Rejected — the backend is internally inconsistent + +**No.** The main-parser folding, `to_regclass`/`search_path` resolution, and +`lower()` under the database default collation all agree, and +`quote_identifier()` is ASCII-conservative like `fmtId()`. The test's passing +`to_regclass('Ä_t') = t` assertion is a live demonstration that the backend +consistently resolves the unquoted high-bit name. + +### Rejected — crash / clean-but-wrong error on an uninitialized `default_locale` + +`pg_downcase_ident()` (`src/backend/utils/adt/pg_locale.c:1369`) explicitly +handles `default_locale == NULL` with an ASCII fallback; it does not call +`pg_newlocale_from_collation(DEFAULT_COLLATION_OID)`, so the `1c77203` +"default locale not initialized" guard is on a different path. A physical +walsender (no database ⇒ `default_locale == NULL`) with identifier-bearing GUCs +carrying a high-bit value connected successfully — no crash, no locale error. + +### Rejected — `Assert(needed == len)` / dangling `default_locale` / truncation rewrite + +The sole caller passes `dstsize = len + 1 > srclen`, and every `downcase_ident_*` +returns `srclen`, so the asserts always hold (tested to 10000-byte and invalid- +UTF8 identifiers). `default_locale` lives in `TopMemoryContext`, set once, never +freed. Follow-ups `e615da8` ("Fix for loop variables") and `6d22c67` ("Don't +accept length of -1 in pg_locale.h APIs") already tidied the length-type nits. +No user-visible effect. + +### Rejected — ASCII semantics before `default_locale` init leak into GUC identifier lists + +The commit message flags that identifiers downcased before +`init_database_collation()` use ASCII. Not observable: `search_path` and peer +list-GUCs store the raw string and re-split via `SplitIdentifierString` at +runtime (after init); runtime `SET`, `ALTER DATABASE/ROLE … SET`, `PGOPTIONS`, +and `postgresql.conf` delivery all agreed in testing. + +### Rejected — familiar libc-database psql/pg_dump client-locale mismatch + +Real but **explicitly pre-existing** (criterion 3): the backend libc folding +result is unchanged by 87b2968; the divergence is the frontend using a different +`LC_CTYPE`, and is documented in `psqlscanslash.l`. Not attributable. + +### Documentation + +`doc/src/sgml/syntax.sgml` says unquoted identifiers may contain "letters with +diacritical marks and non-Latin letters" and are "always folded to lower case", +but says nothing about locale/encoding dependence or an ASCII-only fallback. The +docs are silent/permissive, so master violates no documented behavior. No +documentation defect attributable to 87b2968. + +### Follow-up commits between 87b2968 and master (touched files) + +`git log --oneline 87b2968..master -- src/backend/parser/scansup.c +src/backend/utils/adt/pg_locale*.c src/include/utils/pg_locale.h +src/port/pgstrcasecmp.c` (newest first): + +``` +2cf212d Generate unicode_limits.h. +4854b23 Require ICU 55 or later +e615da8 Fix for loop variables +e6e08dc pg_locale_libc.c: add missing casts to unsigned char. +dbbeafe pg_locale_libc.c: add guards to ctype methods. +3ab2abc Fix obsolete comment. +07211f6 unicode_case.c: change API to signal UTF8 decoding error. +21ffc27 unicode_case.c: defend against truncated UTF8. +1c77203 Guard against uninitialized default locale. +6d22c67 Don't accept length of -1 in pg_locale.h APIs. +b2869eb Fix integer-overflow and alignment hazards in locale-related code. +bdcb85b Fix callers of unicode_strtitle() using srclen == -1. +06ce97b Fix overrun when comparing with unterminated ICU language string. +905e441 Allow setting the collation strength in ICU tailoring rules +11171fe style: define parameterless functions as foo(void). +d942511 Fix memory leaks in pg_locale_icu.c. +de90bb7 Fix theoretical memory leaks in pg_locale_libc.c. +5ca5f12 Fix accidentally cast away qualifiers +af2d4ca Clean up ICU includes. +c4ff35f ICU: use UTF8-optimized case conversion API +451c439 Update copyright for 2026 +0a90df5 Avoid global LC_CTYPE dependency in pg_locale_icu.c. +``` + +None changes the builtin `.downcase_ident = NULL`, the `ctype == NULL` ASCII +fallback, or the frontend folding. `e615da8` and `6d22c67` are the only touches +to `scansup.c`/the `pg_downcase_ident` API and are cosmetic/robustness only; +`0a90df5` fixed the *ICU* single-byte path (rejected candidate above). + +--- + +## How to reproduce + +### The tested defect (TAP) + +```sh +# In the worktree built with -Dtap_tests=enabled: +meson test -C build --suite setup # once, to populate tmp_install +meson test -C build "psql/040_builtin_high_bit_identifier" --print-errorlogs +``` + +On unmodified master this fails 1 of 3 subtests: `to_regclass(unquoted)` returns +`t` and the `LC_CTYPE=C` control finds the table, but `\d` under +`LC_CTYPE=de_DE.iso88591` returns empty (folded `0xC4`→`0xE4`). It `skip_all`s +where no ISO-8859-1 locale is installed. + +Equivalent stand-alone run against the installed tree: + +```sh +export PATH=/inst/bin:$PATH +export LD_LIBRARY_PATH=/inst/lib +export PG_REGRESS=/build/src/test/regress/pg_regress +export top_builddir=/build +cd src/bin/psql +prove -I ../../../src/test/perl -v t/040_builtin_high_bit_identifier.pl +``` + +### Manual reproduction (shows the divergence directly) + +```sh +initdb -D "$PGDATA" -U postgres --locale-provider=builtin --builtin-locale=C \ + --lc-ctype=de_DE.iso88591 --lc-collate=de_DE.iso88591 --encoding=LATIN1 +pg_ctl -D "$PGDATA" -l "$PGDATA/log" -o "-p $PGPORT -k $PGDATA" start + +printf 'SET client_encoding=LATIN1; CREATE TABLE "\304_t"(umark int);' \ + | psql -X -h "$PGDATA" -p "$PGPORT" -U postgres -d postgres + +# backend resolves the unquoted name: +printf "SET client_encoding=LATIN1; SELECT to_regclass('\304_t');" \ + | psql -X -At -h "$PGDATA" -p "$PGPORT" -U postgres -d postgres # -> <0xC4>_t + +# client under an ISO-8859-1 locale folds 0xC4 -> 0xE4 and does NOT find it: +printf '\\d \304_t\n' | env LC_ALL= LC_CTYPE=de_DE.iso88591 PGCLIENTENCODING=LATIN1 \ + psql -X -h "$PGDATA" -p "$PGPORT" -U postgres -d postgres +# -> Did not find any relation named "Ä_t". +``` diff --git a/src/bin/psql/meson.build b/src/bin/psql/meson.build index 922b284..107873d 100644 --- a/src/bin/psql/meson.build +++ b/src/bin/psql/meson.build @@ -78,6 +78,7 @@ tests += { 't/010_tab_completion.pl', 't/020_cancel.pl', 't/030_pager.pl', + 't/040_builtin_high_bit_identifier.pl', ], }, } diff --git a/src/bin/psql/t/040_builtin_high_bit_identifier.pl b/src/bin/psql/t/040_builtin_high_bit_identifier.pl new file mode 100644 index 0000000..7dd0066 --- /dev/null +++ b/src/bin/psql/t/040_builtin_high_bit_identifier.pl @@ -0,0 +1,146 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Regression test for the frontend/backend identifier-folding contract broken by +# commit 87b2968df0f866aaccb6ba69adf284e3c4a79454 ("downcase_identifier(): use +# method table from locale provider."). +# +# That commit routed backend unquoted-identifier case folding through the +# database locale provider's method table and set the "builtin" provider's +# .downcase_ident to NULL, i.e. ASCII-only folding. It did not touch the +# frontend copy of that logic in src/port/pgstrcasecmp.c / patternToSQLRegex() +# (src/fe_utils/string_utils.c), whose own header comment states: +# +# NB: this code should match downcase_truncate_identifier() in scansup.c. +# +# The frontend still folds a high-bit single-byte identifier byte using the +# client process's libc locale (isupper()/tolower()). In a single-byte (LATIN1) +# database using the builtin provider, whose datctype matches the client's +# LC_CTYPE, the two sides agreed before the commit and can now disagree: the +# backend keeps byte 0xC4 ('A' with diaeresis) distinct from 0xE4 ('a' with +# diaeresis), while an ISO-8859-1 client folds 0xC4 -> 0xE4 locally. As a +# result psql's \d (and \password, and tab completion) can act on the wrong +# object or on no object at all. +# +# This test asserts the fix-direction-agnostic invariant: if the backend +# resolves an unquoted name (to_regclass() is non-null), then psql's \d on the +# same unquoted spelling must show that relation. It fails on unmodified master +# and passes once the frontend folding is made provider-aware / ASCII-only. +# +# The divergence requires the *client* to run under a single-byte ISO-8859-1 +# locale where isupper(0xC4) is true, so the test skips where no such locale is +# installed (and on Windows). + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# The defect only manifests under a single-byte ISO-8859-1 client locale. +if ($windows_os) +{ + plan skip_all => 'high-bit client-locale identifier folding needs POSIX locales'; +} + +my @candidates = + qw(de_DE.iso88591 de_DE.ISO8859-1 en_US.iso88591 en_US.ISO8859-1); +my %have = map { $_ => 1 } split(/\n/, `locale -a 2>/dev/null` || ''); +my ($loc8) = grep { $have{$_} } @candidates; +plan skip_all => 'no single-byte ISO-8859-1 locale installed (tried: ' + . join(', ', @candidates) . ')' + unless defined $loc8; + +# Build the high-bit byte in Perl so the result does not depend on this file's +# own encoding. In ISO-8859-1, 0xC4 is upper-case and folds to 0xE4. +my $Ae = chr(0xC4); + +# Server: builtin provider (ASCII-only identifier folding), LATIN1 encoding, and +# datctype == the client locale we will use below. Matching datctype to the +# client locale makes the test airtight: any divergence is then solely the +# builtin provider folding ASCII-only while the client folds per its locale, not +# an incidental client/server locale mismatch. +my $node = PostgreSQL::Test::Cluster->new('main'); +eval { + $node->init( + extra => [ + '--locale-provider=builtin', '--builtin-locale=C', + "--lc-ctype=$loc8", "--lc-collate=$loc8", + '--encoding=LATIN1', + ]); + 1; +} or do +{ + # A build/platform that cannot initialize this cluster cannot exercise the + # defect; treat it as a skip rather than a failure. + my $err = $@; + plan skip_all => + "initdb with builtin provider, LATIN1, lc-ctype=$loc8 failed: $err"; +}; +$node->start; + +# Create a table whose name begins with the high-bit byte 0xC4. The name is +# quoted, so it is stored verbatim (byte 0xC4) regardless of any folding. +$node->safe_psql('postgres', + "SET client_encoding = LATIN1; CREATE TABLE \"${Ae}_t\" (umark int);"); + +# The backend resolves the UNQUOTED spelling: the builtin provider folds it +# ASCII-only, leaving 0xC4 intact, so to_regclass() finds the table. This is +# the antecedent of the invariant and proves the object is addressable unquoted +# (i.e. the later \d failure is about folding, not a missing object/locale). +my $reg = $node->safe_psql('postgres', + "SET client_encoding = LATIN1; SELECT to_regclass('${Ae}_t') IS NOT NULL;"); +is($reg, 't', 'backend resolves the unquoted high-bit identifier'); + +# Sanity control: a client whose LC_CTYPE does not fold 0xC4 (the C locale) must +# find the same table with \d. This proves the describe path itself works on +# this platform, so a genuine folding divergence (below) is distinguishable from +# a broken setup. +{ + local $ENV{LC_CTYPE} = 'C'; + local $ENV{LC_ALL} = undef; + local $ENV{PGCLIENTENCODING} = 'LATIN1'; + my ($rc, $out, $err) = + $node->psql('postgres', "\\d ${Ae}_t", on_error_stop => 0); + like($out, qr/umark/, + 'control: psql \d finds the relation when the client does not fold 0xC4'); +} + +# The defect: under an 8-bit client locale, psql folds 0xC4 -> 0xE4 locally, so +# \d looks up a name (0xE4_t) the backend never created, while the backend +# resolved 0xC4_t just above. Invariant: to_regclass(unquoted) non-null implies +# \d on that same unquoted spelling shows the relation. +{ + local $ENV{LC_CTYPE} = $loc8; + local $ENV{LC_ALL} = undef; + local $ENV{PGCLIENTENCODING} = 'LATIN1'; + my ($rc, $out, $err) = + $node->psql('postgres', "\\d ${Ae}_t", on_error_stop => 0); + like( + $out, + qr/umark/, + "psql \\d resolves the same relation the backend does " + . "(client LC_CTYPE=$loc8)") + or diag( + "Frontend/backend identifier folding disagree.\n" + . " backend to_regclass(unquoted high-bit name) = '$reg' (relation exists)\n" + . " psql \\d rc = $rc\n" + . " psql \\d stdout = " . _show_bytes($out) . "\n" + . " psql \\d stderr = " . _show_bytes($err) . "\n" + . "On master the builtin backend keeps byte 0xC4 while an ISO-8859-1 " + . "psql folds 0xC4 to 0xE4, so \\d looks up the wrong (nonexistent) name."); +} + +$node->stop('immediate'); + +done_testing(); + +# Render a captured psql byte string safely for diagnostics: non-printable and +# high-bit bytes become \xNN so the message stays plain ASCII. +sub _show_bytes +{ + my ($s) = @_; + return '(empty)' unless defined $s && length $s; + $s =~ s/([^\x20-\x7e])/sprintf('\\x%02x', ord($1))/ge; + return "'$s'"; +}