Render the same parameter names every time a statement is rendered

[simon-mundy]

Problem

Rendering the same Sql object more than once gives different SQL and parameter names each time. With a named-parameter driver and a fresh ParameterContainer per call:

$select = (new Select('t'))
    ->columns(['id', 'n' => new Expression('COALESCE(x, ?)', [0])])
    ->where(['a' => 1, 'b' => 2])
    ->where([new In('c', (new Select('u'))->columns(['id'])->where(['d' => 3]))])
    ->order(new Expression('FIELD(id, ?)', [4]));

$sql->prepareStatementForSqlObject($select); // twice

	First render	Second render
Column expression	:n1	:n2
WHERE	:where1, :where2	:where3, :where4
Sub-select	:subselect1where1	:subselect2where1
Unnamed expression	:expr0Param1	:expr1Param1

This affects anything that renders a statement more than once: calling prepareStatementForSqlObject() twice, getSqlString() followed by a prepare, retries, and statement caches keyed on SQL text. The same drift happens with Update, Insert, Delete, Combine and through platform decorators.

exprNParam names also depended on everything else rendered earlier in the PHP process, so the first render of a new object was not predictable either.

Cause

AbstractSql keeps its naming state on the object, and none of it was reset between renders:

• $instanceParameterIndex kept counting per prefix (where, n, ...).
• $processInfo['subselectCount'] kept growing, so sub-selects were numbered from where the last render stopped.
• A sub-select object kept its own $instanceParameterIndex and processInfo from earlier renders. A Select that had been used as a sub-select also kept its subselectN prefix when it was later rendered on its own.
• processExpression() numbered unnamed expressions with a function-static counter shared by the whole process.

Fix

• New AbstractSql::resetRenderState() sets processInfo back to its defaults and empties instanceParameterIndex. A platform decorator also resets its subject, because it copies the subject's state at the start of each render.
• getSqlString() and AbstractPreparableSql::prepareStatement() call it before building. Nested renders call buildSqlString() directly, so they are not reset part way through.
• processSubSelect() gives each sub-select a fresh processInfo and instanceParameterIndex for this render.
• The static counter is now processInfo['expressionCount']. It is passed into sub-selects and read back afterwards, the same way subselectCount is, so parent and sub-select expressions stay uniquely named and keep today's order.

For a new object in a new process, every name is the same as the current first render. Combine and TableGateway need no changes because they render through these entry points. The phpdb-mysql and phpdb-sqlite decorators only read processInfo['paramPrefix'], which is unchanged.

Tests

test/unit/Sql/RenderStateTest.php (#[Group('unit')]). All 10 cases fail before the fix:

• Data provider (select, update, insert, insert from select, delete, combine, unnamed expressions in parent and sub-select): preparing the same object twice gives identical SQL and getNamedArray(), both equal to today's first-render output.
• getSqlString() between two prepares does not change the names.
• A direct prepare, then two prepares through a platform decorator, give the same names.
• A Select that was earlier rendered as a sub-select has no subselectN prefix when prepared on its own.

phpunit --testsuite "unit test" passes (2192 tests). mago format --check, mago lint and mago analyze all exit 0. Every changed line is covered.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!