Invalid parameter number: number of bound variables does not match number of tokens PDO insert

Question

function mysql_insert($data_array){
    $sql = "insert into `". $this->table_name. '`';

    $array_keys = array_keys($data_array);
    $array_keys_comma = implode(",\n", preg_replace('/^(.*?)$/', "`$1`", $array_keys));

    for($a=0,$b=count($data_array); $a<$b; $a++){ $question_marks .="?,";  }

    $array_values = array_values($data_array);
    $array_values_comma = implode(",", $array_values);

    $sql.= " ($array_keys_comma) ";
    $sql.= " values(". substr($question_marks, 0,-1) .")";

    $prepare = $this->connDB->prepare($sql);
    $insert = $prepare->execute(array($array_values_comma));

}

I want to creat like this universal functions, $data_array-comes from $_POST This function will work for all form. But i dont know what is my wrong :S

score 0 · Answer 1 · edited May 23 '17 at 11:56

0

I don't know what is my wrong

That's quite easy to know: number of bound variables does not match number of tokens.

I want to creat like this universal functions, $data_array-comes from $_POST

Here you go: Insert/update helper function using PDO

edited May 23 '17 at 11:56

Community

1
1

answered Aug 08 '13 at 08:04

Your Common Sense

156,878
40
214
345

i just removed id column now its ok, but i should use id column, what should i do? – Arif Cavadov Aug 08 '13 at 08:44
Dunnot what id you are talking about and why you need is for insert but you can see a working code that is not vulnerable to SQL injection (as opposite to yours) – Your Common Sense Aug 08 '13 at 08:53

score -1 · Accepted Answer · edited May 23 '17 at 10:31

$array_values_comma is a scalar after you implode() the array. So you always pass an array of one element to your execute() function. You should pass $array_values.

Here's how I'd write this function:

function mysql_insert($data_array){
    $columns = array_keys($data_array);
    $column_list_delimited = implode(",", 
      array_map(function ($name) { return "`$name`"; }, $columns));

    $question_marks = implode(",", array_fill(1, count($data_array), "?"));

    $sql = "insert into `{$this->table_name}` ($column_list_delimited) 
      values ($question_marks)";

    // always check for these functions returning FALSE, which indicates an error
    // or alternatively set the PDO attribute to use exceptions

    $prepare = $this->connDB->prepare($sql);
    if ($prepare === false) {
      trigger_error(print_r($this->connDB->errorInfo(),true), E_USER_ERROR);
    }

    $insert = $prepare->execute(array_values($data_array));    
    if ($insert === false) {
      trigger_error(print_r($prepare->errorInfo(),true), E_USER_ERROR);
    }

}

A further improvement would be to do some validation of $this->table_name and the keys of $data_array so you know they match an existing table and its columns.

See my answer to escaping column name with PDO for an example of validating column names.

this function is vulnerable to SQL injection through identifiers. You don't need to check PDO functions but make them throw exceptions — Your Common Sense, Aug 08 '13 at 18:40
@YourCommonSense, that's why I recommended to validate table names and column names. I did mention the option of using exceptions, but not all PHP developers are comfortable using exceptions. Downvoting this answer is unwarranted. — Bill Karwin, Aug 08 '13 at 19:40
That's why my other answer uses *whitelisting* approach (note the obligatory `$fields` parameter) and thus invulnerable — Your Common Sense, Aug 08 '13 at 19:56

Invalid parameter number: number of bound variables does not match number of tokens PDO insert

2 Answers2