Use usual semantics for PDO::FETCH_CLASS constructor arguments
| From: | Gina P. Banyard | Date: | Tue, 21 Jan 2025 05:39:26 +0000 |
| Subject: | Use usual semantics for PDO::FETCH_CLASS constructor arguments | ||
| Groups: | php.internals | ||
| Request: | Send a blank email to internals+get-126215@lists.php.net to get a copy of this message | ||
Hello internals,
For the past few weeks I have been refactoring the PDO extension to reduce the complexity of the
implementation but also lower the memory requirement of the _pdo_stmt_t struct.
And in this process I rediscovered an oddity which prevents removing the usage of the FCI struct
(which takes 64 bytes) from it.
The constructor arguments for the class are passed as a PHP array to PDOStatement::setFetchMode(),
PDOStatement::fetchAll(), and PDOStatment::fetchObject().
However, this array of parameters does not have the same semantics as the $args parameter of
call_user_func_array(). (CUFA for short.)
The reason for this is that it uses a rather unintuitive and obscure Zend API called
zend_fcall_info_args_ex(),
this API sets the arguments of a call by copying each argument one after the other as a positional
argument.
And if a zend_function pointer is provided, it will check if the function argument should be passed
by reference and automatically wrap values into a reference without a warning.
This behaviour is at odds with passing a keyed array to CUFA, where the key will act as a named
argument look-up, and with normal calling semantics where a by-ref argument being passed by value
will trigger an E_WARNING.
Moreover, this API is effectively unused. A SourceGraph search [1] came up with 9 results, where
only 2 of them are relevant:
- The UOPZ extension
- The Zephir project, which allows generating a PHP C extension from PHPesque code
- All other 7 projects are extensions generated by using Zephir
I have submitted PRs to Zephir [2] and UOPZ [3] that have been merged that remove the usage of this
API by instead passing the HashTable pointer of the array as the named_params field of the FCI.
Thus, the only remaining usage of this API I can find is now in PDO.
This conversion of using the named_params field of the FCI for calling the constructor can also be
done for PDO,
allowing the removal of the FCI from _pdo_stmt_t, convert the ctor_arg zval (which takes 16 bytes)
to a HashTable* (taking only 8 bytes), and remove various copies of the constructor arguments.
Doing this causes some minor BC breaks as this aligns the behaviour with CUFA which can be seen in
the latest PR implementation. [4]
It is possible to emulate the previous behaviour, but this requires doing a deep copy of the
constructor arguments, negating that benefit.
To me, these minor BC breaks are acceptable as:
- by-ref arguments in a constructor seems already rare, even more so in an object that represents a
database row
- having an argument array behave differently than CUFA or what you would receive with variadic
arugments is somewhat a language inconsistency.
Thoughts?
For reference, this is effectively the same point I raised in a previous internals thread, which
didn't really get much attention. [5]
Best regards,
Gina P. Banyard
[1] https://sourcegraph.com/search?q=context:global+lang:C+zend_fcall_info_args_ex+-file:zend_API.h+-file:zend_API.c+-file:pdo_stmt.c&patternType=keyword&sm=0
[2] Zephir PR: https://github.com/zephir-lang/zephir/pull/2443
[3] UOPZ PR: https://github.com/krakjoe/uopz/pull/188
[4] https://github.com/php/php-src/pull/17427
[5] https://externals.io/message/118847