ext/pdo: Refactor PDO::FETCH_CLASS to not rely on a FCI and use a HashTable for ctor_arg #17427
Conversation
Girgias
force-pushed
the
pdo-stmt-remove-fci-usage
branch
3 times, most recently
from
702f741 to
2bad75c
Compare
Girgias
force-pushed
the
pdo-stmt-remove-fci-usage
branch
from
2bad75c to
81490b2
Compare
Girgias
force-pushed
the
pdo-stmt-remove-fci-usage
branch
from
81490b2 to
5711354
Compare
Girgias
force-pushed
the
pdo-stmt-remove-fci-usage
branch
from
5711354 to
808b10e
Compare
Girgias
force-pushed
the
pdo-stmt-remove-fci-usage
branch
from
808b10e to
4791772
Compare
| stmt->fetch.cls.ce = cep; | ||
| zval ce_name_from_column; | ||
| fetch_value(stmt, &ce_name_from_column, column_index_to_fetch++, NULL); | ||
| if (Z_TYPE(ce_name_from_column) == IS_STRING) { |
There was a problem hiding this comment.
If the type is not string, then we silently ignore it? That seems not nice... should warn or throw or something
There was a problem hiding this comment.
Annoyingly, due to the previous usage of try_convert_to_string() it would accept integers/null/floats convert them to string, and because no class could possibly exist set the CE to stdClass.
| fci->param_count = 0; | ||
| fci->params = NULL; | ||
|
|
||
| zend_fcall_info_args_ex(fci, ce->constructor, &stmt->fetch.cls.ctor_args); |
There was a problem hiding this comment.
Can you document the new behaviour wrt references in UPGRADING please?
| zval_ptr_dtor(return_value); | ||
| RETVAL_FALSE; |
There was a problem hiding this comment.
Okay let's just make it consistently not use it then as it's a big refactor anyway it seems like a good opportunity to do so.
| zend_argument_value_error(3, "must be empty when class provided in argument #2 ($class) does not have a constructor"); | ||
| return false; | ||
| } | ||
| GC_TRY_ADDREF(Z_ARRVAL(args[1])); |
There was a problem hiding this comment.
Okay without looking too deep again: check why you refcount
There was a problem hiding this comment.
The HashTable comes from userland via the call to POD::setFetchMode()
Girgias
force-pushed
the
pdo-stmt-remove-fci-usage
branch
from
4791772 to
83b8a92
Compare
Commits should be reviewed in order.
This removes the last usage of
zend_fcall_info_args_ex()and basically supersedes #9725, which would allow merging #9723.This also aligns the behaviour of the
ctor_argumentsto be have like CUFA.Some follow-up ideas: