Mysqli improvements

Hi Kamil, I like these proposals, especially error reporting, which I've got caught out with before: https://bugs.php.net/bug.php?id=78932 As to "add bind-in-execute to mysqli", by passing an array of parameters to mysqli_stmt::execute(), this is something I'd really like to see. I just wonder if we could take it a little further, so a query can be executed with parameters with one function/method? which is what I was proposing last week: https://news-web.php.net/php.internals/112618 https://marc.info/?l=php-internals&m=160898181628407 Only because I'd like the "right way" to also be the easiest/fastest way, on the basis that developers just want to get the job done, and the simplest way should be the best way. Today, I still see a lot of this: $name = ($_GET['name'] ?? ''); $sql = 'SELECT * FROM user WHERE name LIKE "' . $name . '"'; $result = $mysqli->query($sql); while ($row = $result->fetch_assoc()) { } Yes, not even using error prone (flawed) escaping - something we can address in "phase 2" :-) I suspect (heavy sarcasm implied) it's because it's so much easier than this monstrosity: $name = ($_GET['name'] ?? ''); $sql = 'SELECT * FROM user WHERE name LIKE ?'; $statement = $mysqli->prepare($sql); $statement->bind_param('s', $name); $statement->execute(); $result = $statement->get_result(); while ($row = $result->fetch_assoc()) { } So I really like how you've avoided the difficult bind_param() method, with it's annoying $types string, and pass-by-reference issue, so it looks a bit more like: $statement = $mysqli->prepare($sql); $statement->execute([$name]); $result = $statement->get_result(); while ($row = $result->fetch_assoc()) { } I really hope you can do this - as it allows the developer to re-issue the prepared statement by calling execute() again. But I'd still like to go a little bit further, so we can get $result in a single function/method call, like the original (flawed) approach: $result = $mysqli->execute($sql, [$name]); while ($row = $result->fetch_assoc()) { } Craig On Wed, 30 Dec 2020 at 18:33, Kamil Tekiela <tekiela246@gmail.com> wrote:

On Wed, Dec 30, 2020 at 7:33 PM Kamil Tekiela <tekiela246@gmail.com> wrote:

This RFC appears to cover mostly unrelated topics: Enable exception error reporting by default: Yes, we should definitely do this. Especially as we did the corresponding PDO change. I think people often don't fully appreciate that in mysqli errors can occur all the way down -- it's not enough to check whether your prepare and execute succeed, you might still get an error during an individual fetch... Bind in execute: Sounds like a good idea. I also glanced over your pull request, and it looks like supporting this is very straightforward. 3onnect_error property: While these probably should have been static methods (I don't think they can really be static properties from a technical perspective), I'm not sure this is a problem worth addressing. If exception mode is enabled by default, why would one ever want to use these properties? Under `''new mysqli'' and ''mysqli_connect()'' are not true aliases`, your point 3 seems like an unneeded rant. When the manual lists a constructor and a procedural equivalent as aliases, the relationship is always the same. Similar to how the documentation can list a property as an alias of a function, even though they aren't technically the same -- duh, who would've thought. Regarding your proposal to clean up the construction mess: * Removing mysqli::init() sounds reasonable to me. I'm having a really hard time coming up with a case where it would even do something sensible. Possibly if you do $mysqli->close() and then want to re-initialize the object? Very dubious... * Aligning mysqli_connect() and new mysqli() without arguments. I don't think we should do this. There are two ways to establish a connection: mysqli_init() + mysqli_real_connect(), or just standalone mysqli_connect(). Allowing mysqli_connect() + mysqli_real_connect() as well doesn't seem like an improvement to me. Really, the "new mysqli()" exception without args should not have existed, instead a static "mysqli::init()" method should take that role. Then "new mysqli" would really be equivalent to "mysqli_connect" and "mysqli::init()" would really be equivalent to "mysqli_init()." I think that rather than removing mysqli->init(), it may make more sense to convert it into a static method. We probably can't remove the "new mysqli" zero args exception for BC reasons, but this would at least make everything else sane and nicely symmetric between procedural and OO style. * Aliasing connect and real_connect. This seems okay to me. Again, the intended usage here is "connect" or "init + real_connect", but I don't see an immediate reason why making this "init + connect" wouldn't work. (Allowing the end-user to specify client capability flags seems pretty dubious, but that's an unrelated issue...) * Deprecating mysqli_set_opt. IMHO the mysqli_set_opt name is better than the mysqli_options one. We only have the latter because it happens to be called mysql_options in the underlying C API. I wouldn't deprecate this alias. libmysqlclient support: This is a tricky question. I have spent some time improving PDO MySQL compatibility with libmysqlclient and our test suite now passes with it. However, the state of mysqli is much worse. We have many failing tests, and also many functions/methods that are only implemented if mysqlnd is used. A possibility here is to drop support for libmysqlclient in mysqli and only support it in PDO, where we have a much smaller API surface to deal with. Regards, Nikita

On Wed, 30 Dec 2020 at 18:33, Kamil Tekiela <tekiela246@gmail.com> wrote:

Hi Kamil, As there is a discussion about MySQLi raising exceptions, and you're looking at the general mysqli API... When using mysqli_stmt::bind_param(), it won't raise an exception when the number of variables doesn't match the number of parameters, instead it's a Fatal Error: $statement = $db->prepare('SELECT * FROM user WHERE id = ?'); $statement->bind_param('ii', $id, $id); So database abstractions need to do messy things like: if (count($ref_values) != $statement->param_count) { throw new mysqli_sql_exception('Invalid parameter count', 2034); } else { array_unshift($ref_values, $ref_types); call_user_func_array([$statement, 'bind_param'], $ref_values); } Should mysqli_stmt::bind_param() be updated to raise an exception instead? Or, because it's such a broken function (e.g. passing variables by reference, and the annoying $types string), should we just forget about it, and focus on getting mysqli_stmt::execute() to accept an array of parameters? Craig On Wed, 30 Dec 2020 at 18:33, Kamil Tekiela <tekiela246@gmail.com> wrote: