Remove all redundant references

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela

[kamil-tekiela]

This PR removes call_user_func_array which was necessary in PHP 5. The references magic was only needed for call_user_func_array. The only reference needed is for rowBindedValues which is because PHP cannot handle references to properties the same way as it does with variables. In the future version, you can get rid of this completely after switching to get_result.

[HLeithner]

First thanks for your pull request. I added some comments.

I didn't tested if the reference with mysql statement object but tested it with user defined variable-length argument lists parameter which worked fine.

Can you elaborate what you mean with

after switching to get_result

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

[HLeithner]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.

Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

[kamil-tekiela]

Can you elaborate what you mean with

after switching to get_result

Right now, composer says that PHP 8.1 is the minimum required version. From PHP 8.2 the get_result becomes always available. Until then, you cannot make the switch as it could potentially break code for people who are running mysqli compiled with libmysqlclient.
Once you make the switch to PHP 8.2, you can replace the current code because you do no use result binding in reality. You just need it because there is no alternative. Using get_result will simplify the code and remove the workaround left by me.

I think I still don't get you sorry, which function is new in php 8.2? get_result on the statement exists since 5.3 and no changes are in the function description. Also we use bind_result if I have checked the code correctly.

There is no new function. As you can see on https://www.php.net/manual/en/mysqli-stmt.get-result.php in the note box:

Available only with mysqlnd.

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

[HLeithner]

And if you recall my RFC that was implemented in PHP 8.2 https://wiki.php.net/rfc/mysqli_support_for_libmysql the feature to compile mysqli with libmysqlclient was dropped. So since PHP 8.2, you can safely use get_result() knowing that it will be available on all user platforms.

My whole point is that you don't need bind_result and your code only uses it because get_result might not have been available. In the future, you can refactor the code and remove bind_result.

ok, now I get it, thanks. would be useful for the next major release. If you like I to create a pr I can create a new branch for it.

[richard67]

@HLeithner GitHub still shows that you requested changes, but the review comments are all resolved. Could you review again and approve if ok? Thanks in advance.

[rdeutz]

code review

[richard67]

@kamil-tekiela I've allowed myself to fix the 2 conflicts in the src/Mysqli/MysqliStatement.php file caused by the previously merged PR #349 . Please check and report back if I've done that right. Thanks in advance.

[kamil-tekiela]

Sorry for the delay. Yes, that is correct.

[HLeithner]

Thanks @kamil-tekiela