CODEWITHSUNDEEP
phppostgresql
Joseph
Joseph

Reputation: 3959

pg_query_params fails with too many arguments

I'm not sure why this is failing, I have two examples, they are the same function, but one works and the other doesn't work. I really want the one that doesn't work to work, because it will allow me to put a lot of my potentially repetitive code into a single function.

This calls either setItemWorks or setItemDoesNotWork.

public function setitemName($itemId, $name){
    /* Add the name of the item to the database */

    $fieldName = 'name';
    $tableName = "items";
    $idName = "id";

    $result = $this->setItemWorks($itemId, $name);
    // Use comments to enable below function and disable function call above.
    //$result = $this->setItemDoesNotWork($tableName, $idName, $fieldName, $itemId, $name);

}

setItemDoesNotWork:

private function setItemDoesNotWork($table, $id, $field, $itemId, $fieldValue){

    $_1 = $itemId;
    $_2 = $fieldValue;
    $_3 = $field;
    $_4 = $table;
    $_5 = $id;

    $parameters = array($_1, $_2, $_3, $_4, $_5);

    // If the ID already exists, then update the name!
    $sql = 'update $4 set $3 = $2 where $5 = $1;';
    pg_query_params($this->database, $sql, $parameters);

    // Add ID and Name into table.
    $sql = 'insert into $4($5, $3) select $1, $2 where not exists(select 1 from $4 where $5=$1)';
    $result = pg_query_params($this->database, $sql, $parameters);

    return $result;
}

setItemWorks:

private function setItemWorks($table, $id, $field, $itemId, $fieldValue){

    $_1 = $itemId;
    $_2 = $fieldValue;

    $parameters = array($_1, $_2);

    // If the ID already exists, then update the name!
    $sql = 'update items set $name = $2 where id = $1;';
    pg_query_params($this->database, $sql, $parameters);

    // Add ID and Name into table.
    $sql = 'insert into items(id, name) select $1, $2 where not exists(select 1 from items where id=$1)';
    $result = pg_query_params($this->database, $sql, $parameters);

    return $result;
}

It seems that when I try putting 5 variables into the pg_query_params it doesn't work.

This is the error I am getting:

Error:

[20-Mar-2015 00:17:19 UTC] PHP Warning:  pg_query_params(): Query failed: ERROR:  syntax error at or near "$4"
LINE 1: update $4 set $3 = $2 where $5 = $1;
               ^ in /home/ubuntu/workspace/lib/ItemDatabase.php on line 139

Edit: The current answer I have put down is unsafe, any ideas how to make this safe whilst not being able to use pg_query_params?

Upvotes: 1

Views: 2299

Answers (3)

Erwin Brandstetter
Erwin Brandstetter

Reputation: 658767

You are opening two cans of worms at once:

  • Dynamic SQL while avoiding SQL injection
  • UPSERT while solving race conditions with concurrent transactions

Both are tricky and the best solution depends on the details of your use case.

PL/pgSQL function

I suggest a server-side plpgsql function that encapsulates a clean solution for both problems:

  • SQL injection is impossible.
  • Concurrency issues are resolved.

CREATE OR REPLACE FUNCTION f_dynamic_upsert
         (_tbl text, _id text, _fld text, _item_id int, _field_val text)
  RETURNS void AS
$func$
DECLARE
   _found int;
BEGIN
LOOP
   EXECUTE format('UPDATE %I SET %I = $1 WHERE %I = $2', _tbl, _fld, _id)
   USING   _field_val, _item_id;

   GET DIAGNOSTICS _found = ROW_COUNT;
   IF _found > 0 THEN RETURN; END IF;

   BEGIN
      EXECUTE format('INSERT INTO %I(%I, %I) SELECT $1, $2', _tbl, _fld, _id)
      USING   _field_val, _item_id;
      RETURN;
   EXCEPTION WHEN unique_violation THEN
      -- rarely happens, keep trying
   END;
END LOOP;
END
$func$  LANGUAGE plpgsql;

Call:

SELECT f_dynamic_upsert('foo', 'foo_id', 'foo_val', 3, 'new foo');

Or with named parameter syntax:

SELECT f_dynamic_upsert(_tbl := 'foo'
                      , _id := 'foo_id'
                      , _fld := 'foo_val'
                      , _item_id := 3
                      , _field_val :='new foo');

SQL Fiddle demonstrating it works.

More about dynamic SQL and SQL injection:

Work for the Postgres UPSERT implementation is ongoing: INSERT .. ON CONFLICT UPDATE. With any luck, this will be in then next release 9.5. Details in the Postgres Wiki.
More about UPSERT:

PHP function

Could look something like this:

private function setItem($table, $id, $field, $itemId, $fieldValue) {
   $parameters = array($table, $id, $field, $itemId, $fieldValue)
   $sql = 'SELECT f_dynamic_upsert($1, $2, $3, $4, $5)';

   $result = pg_query_params($this->database, $sql, $parameters);
   return $result;
}

Upvotes: 2

axiac
axiac

Reputation: 72356

The number of parameters does not cause any issue with pg_query_params() (not with only 5 parameters, anyway), the source of the problem is your query.

As the error message says, the query parser does not expect a parameter at that position but a table name.

Query parameters can be used only instead of literal values (strings, numbers, NULL etc). Database, table and field names, SQL keywords, functions, operators and other syntax elements cannot be replaced by parameters.

You have to put the table names and fields in the query. This is why your method setItemWorks() runs and setItemDoesNotWork() fails.

There is a very good reason why pg_query_params() (and similar functions for other DBMS-es) accepts parameters only instead of literal values: the query using parameters is parsed by the server and it has to be correct SQL.

The main purpose of the prepared queries is to do the SQL parsing only once when the same query is executed multiple times using different literal values. An additional advantage of the prepared queries is the protection against SQL injection. Because the values of the parameters are not directly inserted (quoted or not) in the query, the SQL injection is excluded. The values of the parameters are sent unescaped to the server; there is no reason to escape them because they do not become part of an SQL query any more.

Upvotes: 1

Joseph
Joseph

Reputation: 3959

As per wildplasser's comment, I have assembled the function as follows and it now works fine:

private function setItemDoesNotWork($table, $id, $field, $itemId, $fieldValue){

    $_1 = $itemId;
    $_2 = $fieldValue;
    $_3 = $field;
    $_4 = $table;
    $_5 = $id;

    $parameters = array($_1, $_2);

    // If the ID already exists, then update the name!
    $sql = 'update ' . $_4 . ' set ' .$_3 . ' = $2 where ' . $_5 . ' = $1;';
    /*$result = */pg_query_params($this->database, $sql, $parameters);

    // Add ID and Name into table.
    $sql = 'insert into ' . $_4 . '(' . $_5 . ',' . $_3 . ') select $1, $2 where not exists(select 1 from ' . $_4 . ' where ' . $_5 . '=$1)';

    $result = pg_query_params($this->database, $sql, $parameters);

    return $result;
}

Edit:

So this is unsafe, is there an easy way to make this function safe from SQL injection?

Upvotes: 0

Related Questions