How to Generate an SQL Update Statement From an Array

So basically I have a Database class in which you can do the basic operations (Create, Read, Update and Delete) and this is what I came up with for the update method:

public function updateById($table, $id, $updates = []): void
{
        // Generate the SQL statement based on the updates array
        // e.g: updateById('test', 1, [
        //     'column1' => 'newValue1',
        //     'column2' => 'newValue2'
        // ]);

        $st = "UPDATE $table SET ";
        $keys = array_keys($updates);
        $values = array_values($updates);

        for ($i = 0; $i < count($keys); $i++) {
            if ($i != count($keys) - 1) {
                $st .= $keys[$i] . "='" . $values[$i] . "', ";
            } else {
                $st .= $keys[$i] . "='" . $values[$i] . "' ";
            }
        }

        $st .= "WHERE id=$id";

        $query = $this->connection()->prepare($st);
        $query->execute();
}

It does the trick but it looks messy and inefficient. Is there any way I can improve this piece of code?

>Solution :

NOTE

As YourCommonSense pointed out in the comments, this answer is NOT completely SQL injection safe. https://phpdelusions.net/pdo/sql_injection_example

The Answer

I think it’s easier to work with arrays than strings.

This code should do what you want, in a more secure way.

public function updateById($table, $id, $updates = array())
    {
        
        //loop over update fields to generate the SET clause
        $return_arr = [];
        foreach(array_keys($updates) as $key) {
            $return_arr[] = "`{$key}`=:{$key}";
        }
        $set_clause = implode(', ', $return_arr);
        
        //add the id to the query. NOTE: MUST BE DONE AFTER THE ABOVE LOOP
        $updates['id'] = $id;
        
        //prepare the query
        $query = $this->connection()->prepare("UPDATE {$table} SET {$set_clause} WHERE id=:id");
        
        //execute the query, pass named parameters to execute.
        $query->execute($updates);
    }

Also, I would pass a key->value pair for $id as well, so you can update by any or multiple different columns as well.

updateTable($table, $update_ids, $updates = array())
    {
        
        //in this version, we will use anonymous parameters, because the WHERE clause may share the same keys as the SET clause
        //which could sometimes cause unintended actions
        $params = [];
        
        
        //loop over update fields to generate the SET clause
        $set_clause_parts = [];
        foreach($updates as $key => $value) {
            $set_clause_parts[] = "`{$key}`=?";
            $params[] = $value;
        }
        $set_clause = implode(', ', $set_clause_parts);
        
        //add the ids to the query. NOTE: MUST BE DONE AFTER THE ABOVE LOOP
        $where_clause_parts = [];
        foreach($update_ids as $key => $value) {
            $where_clause_parts[] = "`{$key}`=?";
            $params[] = $value;
        }
        $where_clause = implode(', ', $where_clause_parts);
        
        //prepare the query
        $query = $this->connection()->prepare("UPDATE {$table} SET {$set_clause} WHERE {$where_clause}");
        
        //execute the query, pass named parameters to execute.
        $query->execute($params);
    }

NOTE

The $table variable is NOT sql injection proof. This is probably okay if the value does not come from user submitted data. If it IS somehow from user submitted data, such as an HTML form, you should probably verify that it is a valid table in your database. I usually do this by running a query that returns all the table names in my database and checking against it.