Skip to content

Array conversion rewrite, expanded test coverage, and a tested cookbook#13

Merged
jdatcmd merged 3 commits into
masterfrom
array-hardening
Jul 5, 2026
Merged

Array conversion rewrite, expanded test coverage, and a tested cookbook#13
jdatcmd merged 3 commits into
masterfrom
array-hardening

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Three related quality improvements, each validated with the full regression suite on PostgreSQL 11 through 18 (21/21 tests on every version).

1. Array conversion rewritten (fixes three 20-year-old FIXMEs)

  • Output: string elements are now quoted and escaped per array-literal rules (an embedded " used to corrupt the literal), and a PHP null element becomes a SQL NULL instead of ERROR: unrecognized element type.
  • Input: replaced the brace-rewrite + zend_eval_string hack with a real recursive-descent parser. Unquoted text elements ({foo,bar}) used to crash as undefined PHP constants; quoted elements were never dequoted; and table data flowed through eval(). Numeric elements still arrive as PHP int/float, matching the old semantics.
  • Detection: array arguments are recognized from the declared type (arg_is_array in the proc descriptor) instead of "output text starts with {" — a text argument whose value starts with a brace is no longer misparsed.

The shared test's 'fourth' case had this bug baked in (it passed a text value written in PHP array syntax, relying on the eval); it now pins the corrected semantics, with a new case sharing a real text[] through $_SHARED.

New arrays test: NULL elements both directions, quote/backslash/comma/brace/space round trips (base64-verified), unquoted text arrays, numeric type preservation, empty and multi-dimensional arrays, and bytea round trips.

2. Coverage for previously-untested paths

New coverage test pins: plphp.start_proc failure + recovery, DML/utility status and row counts through spi_exec, trigger arguments via $_TD['args'], cursor death across spi_commit and across rolled-back subtransaction()s (both return false, no crash), multibyte UTF-8, TOAST-sized values, PHP try/catch, and DO-block runtime errors.

Also removes sql/prepare.sql: never in REGRESS, misnamed (no prepared statements — compat.sql owns those), collides with other tests' objects, and its one unique pattern (pre-return_next whole-array SRFs) silently returns zero rows on current code.

Known limitation surfaced (not fixed here): procedures with INOUT parameters fail with "function returning record called in context that cannot accept type record".

3. A tested cookbook

doc/cookbook.md — recipes that showcase the PHP stdlib: filter_var CHECK constraints, bcrypt via password_hash, HMAC tokens, recursive JSON reshaping, regex SRFs, a generic JSON-diff audit trigger (plan cached in $_SHARED), batch processing with periodic commits, streaming scans that stop early, CSV/XML shredding, zlib compression. Every recipe in the "Tested recipes" section runs verbatim in the new cookbook regression test, so the docs can't rot. Side-effecting recipes (webhooks, mail(), slugify) are doc-only with caveats.

🤖 Generated with Claude Code

ChronicallyJD and others added 3 commits July 5, 2026 15:07
Both directions of array conversion had 20-year-old FIXMEs:

- plphp_convert_to_pg_array emitted string elements with no escaping
  (an embedded quote corrupted the literal) and errored out on PHP
  null elements.  Now strings are quoted/escaped per array-literal
  rules and null becomes a SQL NULL element.

- plphp_convert_from_pg_array rewrote braces to "array(" and fed the
  data to zend_eval_string.  Unquoted text elements crashed as
  undefined PHP constants, quoted elements were never dequoted, and
  table data flowed through eval().  Replace it with a recursive
  descent parser over the array output form that handles quoting,
  escapes, NULLs, and nested arrays, preserving the numeric element
  types (int/float) the eval-based code produced.

- Argument arrays were detected by "output text starts with {"; a
  text argument whose value starts with a brace was misparsed as an
  array.  Detect from the declared argument type instead
  (arg_is_array in plphp_proc_desc).

New "arrays" regression test covers NULL elements both directions,
quote/backslash/comma/brace/space escaping round trips, unquoted text
arrays, numeric type preservation, empty and multi-dimensional
arrays, and bytea round trips.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New "coverage" regression test pins: start_proc failure and recovery,
DML/utility statement status and row counts through spi_exec (and that
fetching/rewinding a non-SELECT result is refused politely), trigger
arguments via $_TD['args'], cursor death across spi_commit and across
a rolled-back subtransaction() (both return false rather than
crashing), multibyte UTF-8 round trips, TOAST-sized values, PHP
try/catch in ordinary function code, and DO block runtime errors.

sql/prepare.sql was never in REGRESS, is misnamed (it contains no
prepared statements; compat.sql owns those), collides with objects
from other tests, and its one unique case -- the pre-return_next
"return the whole result array" SRF style -- silently returns zero
rows on the current code, so there is nothing left worth keeping.

Known limitation surfaced while writing this (not pinned because the
error text varies): procedures with INOUT parameters fail with
"function returning record called in context that cannot accept type
record".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
doc/cookbook.md shows where PL/php earns its keep over plpgsql: the
PHP standard library. Recipes: filter_var CHECK constraints, bcrypt
via password_hash (no pgcrypto), HMAC-signed tokens, recursive JSON
reshaping, regex set-returning functions, a generic JSON-diff audit
trigger with its plan cached in $_SHARED, batch processing with
periodic commits, streaming scans that stop early via spi_query,
CSV and XML shredding, and zlib compression.

Every recipe in the "Tested recipes" section is exercised verbatim by
the new "cookbook" regression test, so the page cannot silently rot.
Side-effecting recipes (HTTP webhooks via stream wrappers, mail(),
locale-dependent slugify) are documented separately with caveats about
running inside the backend's transaction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants