TalkPHP - What's wrong?

TalkPHP (http://www.talkphp.com/forums.php)

- Absolute Beginners (http://www.talkphp.com/absolute-beginners/)

- - What's wrong? (http://www.talkphp.com/absolute-beginners/1189-whats-wrong.html)

Tanax

09-20-2007 11:02 AM

What's wrong?

PHP Code:


		
			
public function save() {
        
            
            if(isset($this->data['u_id'])) {
                
                // UPDATE query on database
                $this->db->query("UPDATE ".$this->table['members']." (`u_id`, `username`, `password`, `registerdate`, `registerip`) VALUES ('".$this->data['u_id']."', '".$this->data['username']."', '".$this->data['password']."', '".$this->data['registerdate']."', '".$this->data['registerip']."') WHERE `u_id` = '".$this->data['u_id']."'");
        
            }

            else {
                
                // INSERT query on database
                $this->db->query('INSERT INTO ' . $this->table['members'] . ' VALUES (NULL, ' . $this->data['username'] . ', ' . $this->data['password'] . ', ' . $this->data['registerdate'] . ', ' . $this->data['registerip'] . ')');
            
            }
    
        }

Gives me this:

Code:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '(`u_id`, `username`, `password`, `registerdate`, `registerip`) VALUES ('', 'Tana' at line 1

Wildhoney

09-20-2007 11:11 AM

MySQL UPDATE queries are written differently to INSERT queries. An UPDATE query looks like so:

Code:

UPDATE

        myTable

SET

        MyId = 5,

        myUsername = 'Wildhoney'

WHERE

        myColumn = 'Foo'

Tanax

09-20-2007 11:29 AM

PHP Code:


		
			
if(isset($this->data['uid'])) {
                
                // UPDATE query on database
                $this->db->query('UPDATE '.$this->table['members'].' SET username = '.$this->data['username'].', password = '.$this->data['password'].', registerdate = '.$this->data['registerdate'].', registerip = '.$this->data['registerip'].' WHERE uid = '.$this->data['uid']);
        
            }

Still gives me:

Quote:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE uid =' at line 2

Karl	09-20-2007 11:38 AM

May I first suggest the use of sprintf to improve readability. As for the query, it seems fine except for the lack of quotes around strings. Try this:

PHP Code:


		
			
$szSql = sprintf("    UPDATE
                        %s 
                    SET 
                        username = '%s', 
                        password = '%s', 
                        registerdate = '%s', 
                        registerip = '%s' 
                    WHERE 
                        uid = %d",
                    $this->table['members'],
                    mysql_escape_string($this->data['username']),
                    mysql_escape_string($this->data['password']),
                    $this->data['registerdate'],
                    $this->data['registerip'],
                    $this->data['uid']);

$this->db->query($szSql);

On another note, how much nicer does that code look? :)

Tanax

09-20-2007 11:41 AM

Well, the mysql_escape_string is a bit unneccessary, due to this:

PHP Code:


		
			
public function setName($name) {
            
            $name = $this->db->makesafe($name);
            $this->data['username'] = (string) $name;
    
        }

PHP Code:


		
			
public function makesafe($string) {
            
            $result = mysql_escape_string($string);
            
            return $result;
            
        }

But I'll try with the the other things you mentioned! :D

Tanax

09-20-2007 11:58 AM

Okey, I got it working without any errors now..

PHP Code:


		
			
public function save() {
        
            
            if(isset($this->data['uid'])) {
                
                // UPDATE query on database
                $sql = sprintf("    UPDATE
                                        %s 
                                    SET 
                                        username = '%s', 
                                        password = '%s', 
                                        registerdate = '%s', 
                                        registerip = '%s' 
                                    WHERE 
                                        uid = %d",
                    
                                        $this->table['members'],
                                        $this->data['username'],
                                        $this->data['password'],
                                        $this->data['registerdate'],
                                        $this->data['registerip'],
                                        $this->data['uid']);
        
            }

            else {
                
                // INSERT query on database
                $sql = sprintf("    INSERT INTO
                                        %
                                    VALUES
                                        uid = NULL,
                                        username = '%s',
                                        password = '%s',
                                        registerdate = '%s',
                                        registerip = '%s'",
                                        
                                        $this->table['members'],
                                        $this->data['username'],
                                        $this->data['password'],
                                        $this->data['registerdate'],
                                        $this->data['registerip']);
            
            }
            
            $insert = $this->db->query($sql);
            if($insert) {
                
                return true;
                
            } else {
                
                return false;
                
            }
    
        }

PHP Code:


		
			
$system->user = new member($system->db, 'users');
    $system->user->setName('Tanax');
    $system->user->setPass('pass');
    $system->user->setRegdate(time());
    $system->user->setRegip($_SERVER['REMOTE_ADDR']);
    if($system->user->save()) {
        
        echo '<br />Admin account created!';
        
    } else {
        
        echo '<br />' .mysql_error();
        
    }

It printed "Admin account created, however.. when I check the database, no users exists..

Karl	09-20-2007 12:20 PM

OK, the better way to achieve this is to frist execute the INSERT query which will fail on duplicates (assuming of course, you have approprirate PK/unique fields). You then simply determine if the last INSERT actually inserted any data (using mysql_affected_rows), finally, if no data was inserted, you can simply UPDATE the user record. The code should look like this:

PHP Code:


		
			
Execute INSERT query

if (mysql_affected_rows() == -1)
{
       Execute UPDATE query
}

Tanax

09-20-2007 12:30 PM

Whoa, too advanced for me karl :|

or what, I use that code that you gave, and just insert the UPDATE query inside the if statement and the INSERT query before?

Wildhoney

09-20-2007 12:31 PM

...Or of course if you wanted to be advanced about this then you could put the SELECT and the UPDATE into one MySQL statement.

Code:

INSERT INTO

        myTable

        (myKey, myColumn1)

VALUES

        ('myKey', 'myColumn1')

ON DUPLICATE KEY

UPDATE

        myColumn1 = 'myData1'

That's for future reference. * Stops confusing matters *

Karl	09-20-2007 12:45 PM

Yes you understood that correct, but as Wildhoney has just pointed out, this can be achieved with just one query, which is definately the way to go if your using MySQL 5 or above.

Tanax

09-20-2007 12:57 PM

So in my example, it would look like this?

PHP Code:


		
			
$sql = sprintf("INSERT INTO
    %s
    (uid, username, password, registerdate, registerip)
VALUES
    ('NULL', '%s', '%s', '%s', '%s')
ON DUPLICATE KEY
UPDATE
    username = '%s',
        password = '%s',
        registerdate = '%s',
        registerip = '%s'",
$this->table['members'],
$this->data['username'],
$this->data['password'],
$this->data['registerdate'],
$this->data['registerip']);

$this->db->query($sql);

Karl	09-20-2007 01:08 PM

Almost, just a few minor adjustments.

PHP Code:


		
			
<?php
$sql = sprintf('INSERT INTO
                    %1$s
                    (username, password, registerdate, registerip)
                VALUES
                    ("%2$s", "%3$s", "%4$s", "%5$s")
                ON DUPLICATE KEY
                UPDATE
                    username = "%2$s",
                    password = "%3$s",
                    registerdate = "%4$s",
                    registerip = "%5$s"',
                $this->table['members'],
                $this->data['username'],
                $this->data['password'],
                $this->data['registerdate'],
                $this->data['registerip']);

$this->db->query($sql);  

?>

If the sprintf placeholders are confusing you in this example, I'd suggest reading Salathe's article which explains how to use them properly, the article can be viewed here.

Tanax

09-20-2007 01:37 PM

No no, it's not confusing, it's brilliant :D
Just didn't know of it before ^^

Thanks, it's working :D

All times are GMT. The time now is 07:37 PM.