ext/pdo: Refactor PDO::FETCH_FUNC to not rely on an FCI on the struct #17420
Conversation
ext/pdo/pdo_stmt.c
Outdated
| /* TODO ArgumentCountError? */ | ||
| pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch function specified"); | ||
| return false; | ||
| } | ||
| fetch_function_param_num = stmt->column_count; /* probably less */ |
There was a problem hiding this comment.
Isn't this variable redundant now? You set it to 0 after the call and then use it as idx was before.
There was a problem hiding this comment.
In the follow-up PR, I'm getting rid of idx as it only serves for a check with the SERIALIZE flag
| ZVAL_COPY_VALUE(return_value, &retval); | ||
| } | ||
| } | ||
| zend_call_known_fcc(&stmt->fetch.func.fcc, return_value, fetch_function_param_num, fetch_function_params, NULL); |
There was a problem hiding this comment.
I don't know what the difference is, so someone else will have to check if you're using this correctly. Maybe @nielsdos would like to take a look?
There was a problem hiding this comment.
The difference is just that we no longer store an fci, but that we construct it on the fly. This reduces the memory usage a bit overall.
There was a problem hiding this comment.
I also wonder what happens if the callback returns a reference, but that would be a pre-existing problem I suppose...
ext/pdo/pdo_stmt.c
Outdated
| /* TODO ArgumentCountError? */ | ||
| pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch function specified"); | ||
| return false; | ||
| } | ||
| /* We can probably infer items than stmt->column_count for some cases */ |
There was a problem hiding this comment.
I didn't understand this sentence. Something gramatically makes this difficult for me to understand, I guess the "than" is out of place?
| ZVAL_COPY_VALUE(return_value, &retval); | ||
| } | ||
| } | ||
| zend_call_known_fcc(&stmt->fetch.func.fcc, return_value, fetch_function_param_num, fetch_function_params, NULL); |
There was a problem hiding this comment.
The difference is just that we no longer store an fci, but that we construct it on the fly. This reduces the memory usage a bit overall.
| ZVAL_COPY_VALUE(return_value, &retval); | ||
| } | ||
| } | ||
| zend_call_known_fcc(&stmt->fetch.func.fcc, return_value, fetch_function_param_num, fetch_function_params, NULL); |
There was a problem hiding this comment.
I also wonder what happens if the callback returns a reference, but that would be a pre-existing problem I suppose...
I have a follow-up PR that removes the FCI struct for
fetch.cls.