Skip to content

ext/pdo_pgsql: Fix several lazy fetch defects - #23065

Open
KentarouTakeda wants to merge 4 commits into
php:PHP-8.5from
KentarouTakeda:pdo-pgsql-lazy-fetch-drain
Open

ext/pdo_pgsql: Fix several lazy fetch defects#23065
KentarouTakeda wants to merge 4 commits into
php:PHP-8.5from
KentarouTakeda:pdo-pgsql-lazy-fetch-drain

Conversation

@KentarouTakeda

@KentarouTakeda KentarouTakeda commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

With PDO::ATTR_PREFETCH => 0 a statement streams its result set, and the cleanup reads the rest of it by calling PQgetResult() until it returns NULL. That never happens while the connection is copying: PQgetResult() hands out a fresh COPY result every time. A COPY run through a lazy fetch has therefore spun at 100% CPU since 8.5.0, as soon as another lazy fetch takes the connection over. The copy has to be ended first: a copy in with PQputCopyEnd(), a copy out by draining PQgetCopyData().

The drain was skipped as well, because is_running_unbuffered was cleared first, both in the cleanup's own abort path and in pgsql_stmt_fetch() before it calls the cleanup. With PDO::ATTR_EMULATE_PREPARES or Pdo\Pgsql::ATTR_DISABLE_PREPARES the connection then stayed busy and the next lazy fetch failed with "another command is already in progress". Only the statement the connection still points at cancels and drains. A buffered query drains a pending stream as a side effect, and anything read after that belongs to the next statement.

The connection's pointer to the statement streaming on it was only cleared while closing a server-side prepared statement, which those two modes do not create, so destroying one left the pointer dangling for the next lazy fetch to read. And a statement whose stream was taken over kept its row counters after its result had been freed, so fetch() returned a row of NULLs rather than false.

The COPY fix comes first because the drain fix is what makes the other cleanup paths reach that loop. The pointer fix comes second because the drain tests destroy a statement and execute the next one. Without it they would read the freed statement and crash under an allocator that poisons freed memory. Every test fails, or hangs, on the commit before the one that adds it.

@KentarouTakeda
KentarouTakeda force-pushed the pdo-pgsql-lazy-fetch-drain branch from 1f18350 to df8d70e Compare August 5, 2026 16:13
@KentarouTakeda KentarouTakeda changed the title ext/pdo_pgsql: Drain the connection when a lazy fetch ends ext/pdo_pgsql: Fix the connection state left behind by a lazy fetch Aug 5, 2026
@KentarouTakeda
KentarouTakeda force-pushed the pdo-pgsql-lazy-fetch-drain branch 3 times, most recently from a698573 to 1a30f7e Compare August 5, 2026 16:28
@KentarouTakeda

Copy link
Copy Markdown
Contributor Author

Converting to draft: the drain loop this restores can hang when the statement left the connection in a COPY state. I will update once that is sorted out.

@KentarouTakeda
KentarouTakeda marked this pull request as draft August 5, 2026 17:04
@KentarouTakeda
KentarouTakeda force-pushed the pdo-pgsql-lazy-fetch-drain branch from 1a30f7e to e3fbdf7 Compare August 7, 2026 13:24
@KentarouTakeda KentarouTakeda changed the title ext/pdo_pgsql: Fix the connection state left behind by a lazy fetch ext/pdo_pgsql: Fix several lazy fetch defects Aug 7, 2026
@KentarouTakeda
KentarouTakeda force-pushed the pdo-pgsql-lazy-fetch-drain branch 2 times, most recently from 8fe3542 to 1f17859 Compare August 7, 2026 23:51
@KentarouTakeda
KentarouTakeda force-pushed the pdo-pgsql-lazy-fetch-drain branch from 1f17859 to 7f2e240 Compare August 8, 2026 00:01
@KentarouTakeda
KentarouTakeda marked this pull request as ready for review August 8, 2026 00:19
@KentarouTakeda

Copy link
Copy Markdown
Contributor Author

Sorted out: the COPY hang is now fixed first, and the connection state fixes follow it. The change is four commits and the description is rewritten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant