Give each value of an array expression parameter its own placeholder

[simon-mundy]

Problem

An array passed as an Expression parameter is wrapped in Argument\Values. When the statement is built, AbstractSql::flattenExpressionValues() expands that into one bound value per element, but Expression::getExpressionData() replaced the matching ? with a single %s. The specification and the value list therefore disagreed in length, and the leftover values were consumed by whatever came next in the same predicate set.

(new Select('t'))->where([
    new Predicate\Expression('a IN (?)', [[1, 2, 3]]),
    'b' => 4,
]);

	Before	After
Prepared (named params)	WHERE a IN (:where1) AND :where2 = :where3, the column b is bound as the value 2	WHERE a IN (:where1, :where2, :where3) AND "b" = :where4
getSqlString()	WHERE a IN ('1') AND '2' = '3'	WHERE a IN ('1', '2', '3') AND "b" = '4'

The query is silently wrong rather than failing, and the value 4 is never bound.

Fix

Expression::getExpressionData() now swaps each ? for as many placeholders as its argument will bind:

• Argument\Values (including a plain array) gives a comma-separated list, %s, %s, %s, or NULL when the array is empty. Before this change, an empty array failed in vsprintf().
• Every other argument still gets a single %s.

The surrounding parentheses are left to the expression. That matches the documented usage ('id IN (?)' → id IN (?, ?, ?, ?, ?) in docs/book/sql/advanced.md) and the existing '? IN (?)' test. Adding parentheses automatically would turn every existing IN (?) into IN ((…)).

An array placeholder that is not directly wrapped by ( and ) (whitespace allowed) now throws PhpDb\Sql\Exception\InvalidArgumentException::forUnenclosedValuesPlaceholder(). The message names the expression and points to IN (?). Without the check, 'a IN ?' with [[1, 2, 3]] would render a IN :p1, :p2, :p3, which is invalid SQL. The rule is strict: 'a IN (?, ?)' with [[1, 2], 3] also throws, because the array's ? is followed by a comma rather than ). Scalar placeholders are not checked. The check only applies to ? placeholders, since named placeholders are never expanded.

The named-placeholder fallback check (:\w* count vs parameter count) is unchanged and still runs against the full rendered specification.

Docs

The array example in advanced.md called Argument::value([1, 2, 3, 4, 5]), which throws a TypeError because value() only takes scalars. It now uses Argument::values(), and a short note explains the expansion, the empty-array NULL, and the exception for a placeholder outside its own parentheses.

Tests

• SelectTest::arrayExpressionParameterDoesNotShiftValuesIntoFollowingPredicates: the reproduction above, with a data provider covering both prepared (named parameters, asserting the SQL and the bound values) and unprepared rendering.
• ExpressionTest::getExpressionDataGivesEachArrayValueItsOwnPlaceholder: checks the generated specification for a plain array, an empty array, an array between other parameters (? IN (?)), an explicit Argument\Values, and whitespace inside the parentheses (IN ( ? )). None of these throw.
• ExpressionTest::getExpressionDataRejectsArrayPlaceholderOutsideParentheses: asserts the exception class and message for 'a IN ?' (array, empty array, and Argument\Values), an unmatched ( or ), and 'a IN (?, ?)'.
• InvalidArgumentExceptionTest: the new named constructor is added to the catalogue's provider.

Both failed before the fix. With it, the full unit suite passes (2151 tests), Expression and Sql\Exception\InvalidArgumentException have 100% line coverage, and mago format --check, mago lint and mago analyze are all clean.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!