2

I am making a function to more easily use prepared statements in sql queries. But while working on it, I ran into a strange bug. When you run the code below the first var_dump of $response prints the $stmt variable like I expect it to, but after closing the $stmt it give a lot of Warnings and prints a NULL filled version of $stmt. I only make a copy of $stmt as $response, so I wouldn't expect $response to change when closing $stmt. Could someone explain why this happens and how I can prevent it?

function sql_bug() {
    global $dbc; // Database connection
    $sql = "UPDATE users SET username = 'J0R1AN' WHERE id = 1"; // Test query
    if ($stmt = $dbc->prepare($sql)) {
        if ($stmt->execute()) {
            $response = $stmt->get_result();
            if ($response === false) {
                // Checks for e.g. UPDATE queries that return bool(false) instead of a mysqli_result, 
                // and sets $response to $stmt instead, to return more valuable information when using UPDATE
                $response = $stmt;
            }
            var_dump($response); // Prints expected $stmt variable
            $stmt->close(); // Somehow changes $response?
            var_dump($response); // Prints $stmt variable filled with NULLS
            return $response;
        }
    }
    return false;
}
Dharman
  • 30,962
  • 25
  • 85
  • 135
J0R1AN
  • 583
  • 7
  • 10
  • You need to stop manually checking for errors. Please read: [Should we ever check for mysqli_connect() errors manually?](https://stackoverflow.com/q/58808332/1839439) and [Should I manually check for errors when calling “mysqli_stmt_prepare”?](https://stackoverflow.com/q/62216426/1839439) – Dharman Jul 30 '20 at 18:15
  • If you close the object then what do you expect to happen? Why do you close it? – Dharman Jul 30 '20 at 18:16
  • *"I only make a copy of $stmt as $response"* - No, objects are not copied this way. You would need to use `clone` to copy an object. – Dharman Jul 30 '20 at 18:18
  • Sorry I didn't clarify, I close the statement because I read that that was good practice and lets MySQL know that it can forget the statement. I expect the $response variable (which is a copy of $stmt when it still had content) to stay how it it, so that I can return it and work with the variables after closing the original $stmt – J0R1AN Jul 30 '20 at 18:20
  • Oh I just say your next comment, so if you use the `=` operator on an object it doesn't copy it? Thanks! EDIT: When I run `clone` on the `$stmt` variable, I get this error: `Fatal error: Uncaught Error: Trying to clone an uncloneable object of class mysqli_stmt`. Is there something I have to do to make it 'cloneable'? – J0R1AN Jul 30 '20 at 18:21
  • As you can see closing is a rather bad option in this case. You close the statement or connection when you are 100% sure you do not need it anywhere else in your program. Most of the time you don't need to call this function at all. I never use it. – Dharman Jul 30 '20 at 18:22
  • Let's take a different look at your question. Please, tell us what kind of problem are you trying to solve with this function? Writing such abstraction function is a great idea, but your function has so many problems that you are not improving much as it is. If you can [edit] the question to explain what is the reason for this function and what you want to achieve I could help you write it properly. – Dharman Jul 30 '20 at 18:23
  • Now I really get it, I should just not bother closing the statement at all in this case. Thanks for the help :) – J0R1AN Jul 30 '20 at 18:26

1 Answers1

3

Variable assignment does not make a new copy of an object in PHP. To create a copy you would need to use clone. Simple example:

$obj  = (object) ['a'=>42];
$new_obj = $obj;
$obj->a = 100;

var_dump($new_obj); 
// outputs 
// stdClass Object
// (
//     [a] => 100
// )

You have fallen victim to multiple fallacies.

  1. You do not need to close anything. PHP will do it for you. Using close only complicates a lot of things.
  2. Don't check return value of prepare or execute functions. Instead enable mysqli error reporting. How to get the error message in MySQLi?
  3. Don't use global objects or limit their use to the absolute minimum.
  4. You don't need to expose mysqli_stmt or mysqli_result objects outside of your function. Once you perform the statement and get the data you can discard them.

If we wanted to fix this function properly we could do something like this:

function sql_bug(\mysqli $dbc, string $sql, array $params = []): ?array {
    $stmt = $dbc->prepare($sql);
    if ($params) {
        // bind optional parameters if the query has variable input
        $stmt->bind_param(str_repeat("s", count($params)), ...$params);
    }
    $stmt->execute();
    $response = $stmt->get_result();
    if ($response) {
        // If the query returned results then fetch them into an array and return it
        return $response->fetch_all(MYSQLI_BOTH);
    }
    // return nothing if the query was successfully executed and it didn't produce results
    return null;
}

The above function is a generic function that can handle any SQL statement with and without parameters. It will not return anything if the query was INSERT or UPDATE and if it was SELECT it will return the data in an array. No need to copy or return the inner objects. You are writing a function to abstract from the mysqli innards.

Dharman
  • 30,962
  • 25
  • 85
  • 135
  • Thanks for the comment, I learned a lot from it. But one feature I would like my function to have is return the $stmt variable so that I can for example read the affected_rows of an UPDATE query. I will change your code slightly to make that work – J0R1AN Jul 30 '20 at 19:54
  • There's no need. It is an attribute of mysqli object also. See https://www.php.net/manual/en/mysqli.affected-rows.php – Dharman Jul 30 '20 at 20:04
  • 1
    One thing about that, when I execute the sql query of for example UPDATE and then inside the `sql_bug()` function itself `echo $dbc->affected_rows` I get the expected more than 0 affected_rows, but when I do that same code outside the function, so after I call `sql_bug()` I always get -1 as affected_rows. In the documentation, it says that it will return from the last query but it feels like there is some scope issue I am running into. I also doesn't give an error, as the `error` variable is empty and my UPDATE query works as intended and actually changes the values – J0R1AN Aug 01 '20 at 08:28
  • @J0R1AN Could you ask that as a separate question, please. Create a simple reproducible example using this function. – Dharman Aug 01 '20 at 09:00