Skip to content

fix: keep TableExtractor table names as strings - #1794

Merged
nunomaduro merged 1 commit into
pestphp:5.xfrom
danny-de-wit:fix/tia-table-extractor-numeric-keys
Aug 3, 2026
Merged

fix: keep TableExtractor table names as strings#1794
nunomaduro merged 1 commit into
pestphp:5.xfrom
danny-de-wit:fix/tia-table-extractor-numeric-keys

Conversation

@danny-de-wit

Copy link
Copy Markdown
Contributor

Fixes #1793.

The bug

TableExtractor collects table names as array keys and returns array_keys(). PHP silently coerces numeric-string keys to integers, so the method can return int values despite its declared list<string> — and Recorder::linkTable(string $table) then throws a TypeError under strict_types.

Standard SQL is enough to trigger it, because the numeric operand matches the FROM pattern:

TableExtractor::fromSql('select substring(name from 1 for 3) from users');
// before: [int(1), 'users']
// after:  ['1', 'users']

In our suite one query using substring(external_data->>'RequestedReceiptDate' FROM 1 FOR 10) turned ~20 otherwise-passing tests into TypeErrors. The failure mode is unkind: it surfaces as application test failures, so it reads as a bug in the project under test rather than in the analysis engine.

The fix

Cast on the way out, at both call sites (fromSql() and fromMigrationSource()):

$out = array_map(strval(...), array_keys($tables));

Why cast rather than skip numeric names

Dropping numeric identifiers would also have fixed the crash, but 123 is a legal quoted table name, and skipping it would silently lose a real dependency from the graph. Casting preserves it.

Note this leaves the pseudo-table '1' recorded for substring(x FROM 1 FOR 10). That's harmless — nothing will ever report a change to a table named 1 — but filtering the FROM <number> case out of the extraction regex would be a tidier follow-up, and deliberately out of scope here to keep the fix minimal.

Tests

One regression test per affected call site, added to the existing tests/Unit/Plugins/Tia/TableExtractor.php. Both fail on the current code:

✕ fromSql() → it does not leak int keys for numeric identifiers
  Failed asserting that 1 is of type string.

✕ fromMigrationSource() → it does not leak int keys for numeric table names
  -    0 => '123'
  +    0 => 123

composer test:unit on that file, test:lint (rector + pint) and test:type:check (phpstan) all pass.

PHP coerces numeric-string array keys to ints, so collecting table names in
$tables[$name] and returning array_keys() can yield ints. That breaks the
declared list<string> and throws a TypeError in Recorder::linkTable(string).

Standard SQL like substring(x FROM 1 FOR 10) is enough to trigger it, since
the numeric operand matches the FROM pattern.

Closes pestphp#1793
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.

[Bug]: TIA TypeError - TableExtractor yields int keys for SQL containing "FROM <number>" (e.g. substring(x FROM 1 FOR 10))

2 participants