CODEWITHSUNDEEP
phpmysqlsql
xan
xan

Reputation: 4696

Select and update not working

I have a table viewer with id, ip, date_last_viewed & blog_id as the columns. I'm first checking whether a particular entry having the same IP and blog_id is present or not. If yes, it updates the date. Else, it inserts a new entry.

My code is below:

$search_ip = mysql_query("SELECT ip FROM viewer WHERE ip = '".$_SERVER['REMOTE_ADDR']."' AND blog_id= '".$b_id."' ");

if ($search_ip == false){
    $insert_ip = mysql_query("INSERT INTO viewer (ip, blog_id, date_last_viewed) VALUES ('".$_SERVER['REMOTE_ADDR']."', '".$b_id."', NOW())");
}
else {
    $update_ip = mysql_query("UPDATE viewer SET date_last_viewed = NOW() WHERE ip = '".$_SERVER['REMOTE_ADDR']."' AND blog_id='".$b_id."' ");           
}

The table is not inserting anything. What am I doing wrong here? Also, as I'm new to PHP programming, could someone tell me how to modify the above code to PDO?

Upvotes: 0

Views: 78

Answers (5)

John Woo
John Woo

Reputation: 263733

You can actually do it in just one query.

MySQL has a special feature called INSERT ... ON DUPLICATE KEY UPDATE which basically insert if the record does not exist or update if it already exists. One thing you need to do is to define a unique column(/s)

Based on your statement, you need to define a unique constraint on both column,

ALTER TABLE viewer ADD CONSTRAINT vw_uq UNIQUE (ip, blog_id)

and execute this statement,

INSERT INTO viewer (ip, blog_id, date_last_viewed)
VALUES ($_SERVER['REMOTE_ADDR'], b_id, NOW())
ON DUPLICATE KEY UPDATE date_last_viewed = NOW()

As a sidenote, the query is vulnerable with SQL Injection if the value(s) of the variables came from the outside. Please take a look at the article below to learn how to prevent from it. By using PreparedStatements you can get rid of using single quotes around values.

Upvotes: 5

neelsg
neelsg

Reputation: 4842

$search_ip will never == false, because it is a reference to the result. Use mysql_num_rows($earch_ip) instead. Also note that mysqli replaces this and your code is actually deprecated

Upvotes: 1

user2426701
user2426701

Reputation:

That's not the right way to check if a query returned a value:

$search_ip = mysql_query("SELECT ip FROM viewer WHERE ip = '".$_SERVER['REMOTE_ADDR']."' AND blog_id= '".$b_id."' ");

if (mysql_num_rows($search_ip)==0) {
....
}

Upvotes: 0

Prisoner
Prisoner

Reputation: 27618

Assuming your mysql_query executes correctly, it wont return false. What you should do is check the number of rows it returns. You can do this using mysql_num_rows.

Also, take note of the big red warning box at the top of the mysql_* man pages.

Upvotes: 2

Kermit
Kermit

Reputation: 34054

You should first add error handlers. Then move to mysqli_ and use prepared statements.

$search_ip = mysql_query( "SELECT ... " ) or die( mysql_error() );

if( mysql_num_rows($search_ip) == 0 ) {
     $insert_ip = mysql_query( "INSERT ... " ) or die( mysql_error() );
}
else {
     $update_ip = mysql_query( "UPDATE ... " ) or die( mysql_error() );
}

Upvotes: 1

Related Questions