0

I am trying to write a reusable UPDATE Query in PHP based on Jeffery's INSERT query on laracast

This is Jeff's Insert Query

public function insert($table, $parameters)
{
$sql = sprintf(
        'INSERT INTO %s (%s) VALUES (%s)',
        $table,
        implode(', ', array_keys($parameters)),
        ':' . implode(', :', array_keys($parameters))

    );
    try {
        $statement = $this->pdo->prepare($sql);
        $statement->execute($parameters);
    } catch (Exception $exception) {
        die("Something Went Wrong");
    }
}

This is the Update Code I am Trying to Write

public function update($table, $parameters, $Condition)
{
    $sql = sprintf(
        'UPDATE %s SET %s=%s WHERE ' . $Condition,
        $table,
        implode('=,', array_keys($parameters))
        ,
        ':' . implode(', :', array_keys($parameters))

    );


    try {
        $statement = $this->pdo->prepare($sql);
        $statement->execute($parameters);
    } catch (Exception $exception) {
        die("Something Went Wrong");
    }

}

I want to make it as reusable as the Insert Query by just passing the Data in

All Help is Highly Appricated

Olaitan Adeboye
  • 172
  • 4
  • 21
  • Not sure I understand. You can save the statement as a member of the class instance and reuse it again ONLY if you have the same parameters and values, then you just need to execute the statement again, but with the different parameters – Alon Eitan May 05 '17 at 15:56
  • 3
    Beware, [your code is vulnerable to sql injection](https://phpdelusions.net/pdo/sql_injection_example) – Your Common Sense May 05 '17 at 15:58
  • @AlonEitan it is not a statement but a function he wants to reuse – Your Common Sense May 05 '17 at 15:59
  • @YourCommonSense So I don't understand the "reuse" part - Ignoring the risk in this code that you mentioned, they can pass multidimensional array to the function where each dimension represent the values of a new row to insert - They create the statement once and iterate over the values array and executing it multiple times for each row. Is that not correct? – Alon Eitan May 05 '17 at 16:06
  • @AlonEitan forget PDO and statements. That's just a function. Functions tend to be reused, with different sets of parameters. – Your Common Sense May 05 '17 at 16:10
  • 1
    @YourCommonSense Oh! Like an ORM style function? So is this a duplicate of [this](http://stackoverflow.com/questions/10526190/difference-between-orm-and-pdo) or [this](http://stackoverflow.com/questions/1346457/some-orm-using-pdo)? – Alon Eitan May 05 '17 at 16:21
  • 1
    So you want to write your own query builder and are asking on SO how to do that? This is IMHO "too broad". Just use an existing solution. – Paul Spiegel May 05 '17 at 17:04

2 Answers2

1

This is a really bad idea. Don't do it.

Instead I would recommend creating custom data mappers for your entities, that would contain hand-crafted SQL, with properly bound parameters/values.

But as an experiment, this is what I came up with:

public function update($table, $parameters, $conditions)
{
    $sql = sprintf(
        'UPDATE %s SET %s WHERE %s',
        $table,
        implode(', ',array_map(
            function ($key) {
                return "{$key} = :s_{$key}";
            },
            array_keys($parameters)
        )),
        implode(' AND ',array_map(
            function ($key) {
                return "{$key} = :w_{$key}";
            },
            array_keys($conditions)
        ))
    );

    $parameters = array_combine(
        array_map(function($key){ return ":s_{$key}"; }, array_keys($parameters)),
        $parameters
    ) + array_combine(
        array_map(function($key){ return ":w_{$key}"; }, array_keys($conditions)),
        $conditions
    );

    try {
        $statement = $this->pdo->prepare($sql);
        $statement->execute($parameters);
    } catch (Exception $exception) {
        die("Something Went Wrong");
    }

}

Keep in mind that, is your "users" can somehow affect the keys of either $parameters or $conditions arrays, then this code becomes vulnerable to SQL injection attacks.

P.S.
In your original example, you had $Condition parameter simply concatenated to the end of your query. That would create a huge risk of SQL injection attacks.

tereško
  • 58,060
  • 25
  • 98
  • 150
1

Here is an efficient[1] way to generate UPDATE statements from an array of field names with named parameters that can be executed with arbitrary data (e.g. using PDOStatements).

[1] efficient: It uses sprintf exclusivly to do the interpolation, no concatenation.

Indications:

  • you need to persist some objects, and Doctrine or the likes would be overkill
  • your objects don't have any funky transformations required on their data before saving
  • users will have no opportunity to modify the list of field names (in my case it was hardcoded into the PHP)

We're all adults and can decide for ourselves when the above indications apply.

If you have an associative array, then just take the array_keys of it first.

$tablename = 'character';
$fields = [
    'name',
    'class',
    'hitpoints',
    'battlecry'
];

$first = array_pop($fields);
$fmt = array_reduce($fields, function($carry, $item) {
    return sprintf($carry, sprintf(', %1$s = :%1$s %%s', $item));
}, sprintf(' SET %1$s = :%1$s %%s', $first));

$sql = sprintf('UPDATE %s %s ', $tablename, sprintf($fmt, ';'));

echo $fmt; // UPDATE character SET battlecry = :battlecry , name = :name , class = :class , hitpoints = :hitpoints ;
Julian Suggate
  • 627
  • 1
  • 6
  • 14