Problems with my attempt to implement an UPSERT

Question

I'm having this problem when checking a condition to update a table in PostgreSQL. It has to check if the user download this once and if yes, add +1 in acessos.

<?php
$result2 = pg_query("SELECT * from downloads WHERE (nome = $_POST[nome_download] AND email = $_POST[email_download])");
if (pg_num_rows($result2) == 0){
$result = pg_query("INSERT INTO downloads (nome, email, estado, arquivo, acessos) VALUES ('$_POST[nome_download]','$_POST[email_download]','$_POST[estado_download]','$_SESSION[nome_arquivo_download]','1')");
}else{
$arr[acessos] = $arr[acessos] + 1;
$result = pg_query("UPDATE downloads SET acessos = $arr[acessos] WHERE (nome = $_POST[nome_download] AND email = $_POST[email_download])");
}


if (!$result){
echo "Não foi possível realizar o cadastro. Tente fazer o download mais tarde.";
}
else
{
echo "soft_bd";
pg_close();
}
?>

I'm not sure why it isn't working. Else don't even execute and bugs me that I can't insert the same data in PostgreSQL in case the user downloads this more than one time. — Léo Eduardo Silva
You have a problem with SQL injection too. Never ever paste user input into SQL statement. Also, pg_num_rows can return -1 — user3427419
Have you verified that your Select statement returns the number of rows you expect? — ChrisF♦
I don't know anything about PHP, but I would remove the unnecessary select. Run the update as the first statement. If no row was updated, then run the insert. Or if only a few users download something more than once, then run the insert first, catch the unique key error and do the update. Which one is faster, depends on which happens more often: the update or the insert. If you can upgrade to Postgres 9.5 you can use insert .. on conflict update that does all that in a transactional safe way — a_horse_with_no_name
I was using if (!$result2) and got the same problem. But I'll check this, @ChrisF. Thanks! — Léo Eduardo Silva

Erwin Brandstetter Erwin Brandstetter · Accepted Answer · 2016-01-03T19:52:46

You refer to $arr but it's not evident from your posted code where that is assigned. Either way, if you want to increase the current value of acessos by 1, this approach is completely unsafe in a multi-user environment.

You are also completely open to SQL injection. Use prepared statements instead.

In Postgres 9.5 you can even do this in a single statement with the new UPSERT implementation INSERT ... ON CONFLICT ON ... DO UPDATE - assuming there is a UNIQUE or PRIMARY KEY constraint on (nome, email):

$sql = 'INSERT INTO downloads AS d (nome, email, estado, arquivo, acessos)
        VALUES ($1, $2, $3, $4, 1)
        ON CONFLICT ON (nome, email) DO UPDATE 
        SET    acessos = EXCLUDED.acessos + 1';

For repeated calls, you might use pg_prepare and pg_execute. For a single call use pg_query_params:

pg_query_params($sql, array($_POST[nome_download]
                          , $_POST[email_download]
                          , $_POST[estado_download]
                          , $_SESSION[nome_arquivo_download]));

Problems with my attempt to implement an UPSERT

1 Answers