Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mysqli bind in execute #6271

Merged
merged 17 commits into from
Apr 14, 2021
Merged

Conversation

kamil-tekiela
Copy link
Member

@kamil-tekiela kamil-tekiela commented Oct 4, 2020

Target Version: PHP 8.1

Introduction

PDO has always offered binding values to the prepared statement directly in the execute() call by providing an array with the values. The same functionality was never present in mysqli, but many users have been confused by that lack of seemingly easy functionality. (See Bug #40891, Bug #31096)

Proposal

I would like to propose adding a new optional argument to mysqli_stmt::execute() same as PDO does with PDOStatement::execute(). The goal of this proposal is to simplify mysqli usage with a simple fix which does not require major refactoring.

This proposal tries to address the following mysqli limitations:

// mysqli can only bind by reference and each variable needs to be passed as a separate argument. 
$id = 1;
$name = trim(' Dharman ');
$stmt = $mysqli->prepare('INSERT INTO users(id, name) VALUES(?,?)');
$stmt->bind_param('ss', $id, $name);
$stmt->execute();

// The following would fail and throw an error
$stmt = $mysqli->prepare('INSERT INTO users(id, name) VALUES(?,?)');
$stmt->bind_param('ss', 1, trim(' Dharman '));
$stmt->execute();

// Binding an array can be very confusing
$arr = [2,3,5,8,13];
$stmt = $mysqli->prepare('SELECT name FROM users WHERE id IN ('.str_repeat('?,', count($arr) - 1) . '?)');
$stmt->bind_param(str_repeat('s', count($arr)), ...$arr);
$stmt->execute();

// SOLUTION:  bind in execute 
// it is now possible to bind by value
$stmt = $mysqli->prepare('INSERT INTO users(id, name) VALUES(?,?)');
$stmt->execute([1, trim(' Dharman ')]);

// binding an array becomes less of a chore
$arr = [2,3,5,8,13];
$stmt = $mysqli->prepare('SELECT name FROM users WHERE id IN ('.str_repeat('?,', count($arr) - 1) . '?)');
$stmt->execute($arr);

What about type specifications?

MySQL can type juggle as easily as PHP. The safest way to bind parameters if you are not 100% certain of their type is to bind as a string. In many cases, this is the preferred simplest way. Type specifications should only be used in rare situations when the data should be passed to MySQL with a specific type. In reality, such situations are extremely rare and they depend on the SQL not on PHP data type. For these rare cases, we can continue using bind_param() with the right type specification.

Difference between PDO and mysqli

While the idea came from PDO bind-in-execute implementation, the mysqli proposal differs in two small ways.

  1. Array keys are completely ignored. mysqli doesn't have emulated prepares nor does it have named parameters. Relying on the array keys/indices would make the implementation unnecessarily complex and it would cause unintentional confusion.
  2. Re-binding empty array throws an error in mysqli. PDO simply ignores an empty array and continues to use previously bound values.

libmysql support?

Unfortunately, I am limited to Windows programming and I have no way of developing the same for libmysql and testing it myself. In theory, it should be possible to add this for libmysql with slight adjustments, but support for libmysql is not actively maintained at the moment and there are more problems that would probably need to be addressed by whoever decides to maintain libmysql support.

Backward Incompatible Changes

None that I can find.

@kamil-tekiela kamil-tekiela marked this pull request as ready for review October 22, 2020 10:46
@kamil-tekiela kamil-tekiela force-pushed the mysqli-bind-in-execute branch from 79a3778 to 1850f79 Compare October 30, 2020 14:42
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks basically good to me.

ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
@dragoonis
Copy link
Contributor

Nice work @kamil-tekiela - good DX improvements :-)

UPGRADING Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved
ext/mysqli/tests/mysqli_stmt_execute_bind.phpt Outdated Show resolved Hide resolved
ext/mysqli/tests/mysqli_stmt_execute_bind.phpt Outdated Show resolved Hide resolved
ext/mysqli/mysqli_api.c Show resolved Hide resolved
@kamil-tekiela kamil-tekiela force-pushed the mysqli-bind-in-execute branch from f697abd to 46257dc Compare April 13, 2021 16:06
@kamil-tekiela kamil-tekiela requested a review from nikic April 13, 2021 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants