MySQL ActiveRecord - Protected identifier issue when updating?

[eluser]gRoberts[/eluser]
hey all,

Because the model + active record setup works great for me, I have created a generic Model that all of my models extend.

This generic model exposes the ability to get a single row, all active, and save.

I've come across a problem (probably not related to how I do things) where when updating a record with a column which is a reserved word, it throws an error, yet it allowed me to insert that column in the first place.

For example, I have a column called "Key". This is a reserved word.

Code:

$tmp = new stdClass();
$tmp->SafeColumn = 'test';
$tmp->Key = 'test';
$this->db->insert('table', $tmp);

The above works... Yet:

Code:

$tmp = $this->model->GetSingle(1); // returns a stdClass
$tmp->SafeColumn = 'other value';
$this->db->update('table', $tmp, array('UniqueID' => $tmp->UniqueID));

returns an error

Code:

A Database Error Occurred

Error Number: 1064

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 'Key = 'test', SafeColumn = 'other value' at line 1

UPDATE `table` SET UniqueID = '1', Key = 'test', SafeColumn = 'other value' WHERE `UniqueID` = '1'

Looking at the code, they both use _protect_identifiers to apparently escape the column names, but as you can see above, it isn't.

Have I simply missed a configuration somewhere that automatically handles this?

I've had a look through the DB_active_rec and mysql_driver files, but to be honest, my head is about to explode

I'd really appreciate someone's input on this?

Cheers

Gavin

[eluser]WanWizard[/eluser]
Can you show us the code that causes this error?

Your two examples both show an insert, while the error is an update...

[eluser]gRoberts[/eluser]
Well spotted!

Code:

<?
    if (!defined('BASEPATH')) exit('No direct script access allowed');
    if(class_exists('BaseModel'))
        return;

    class BaseModel extends Model
    {
        private $TableName;
        private $PrimaryKey;
        private $RemovedKey = 'Removed';
        private $Reserved = array('Key');

        private function escape($Input)
        {
            return '`' . $Input . '`';
        }

        public function __construct($TableName, $PrimaryKey, $RemovedKey = 'Removed')
        {
            parent::Model();

            $this->load->database();

            $this->TableName = $this->escape($TableName);
            $this->PrimaryKey = $PrimaryKey;
            $this->RemovedKey = $RemovedKey;
        }

        public function GetSingle($PrimaryID)
        {
            $this->db->where($this->PrimaryKey, $PrimaryID);
            $result = $this->db->get($this->TableName);
            $output = false;
            if($result->num_rows() > 0)
                $output = $result->row();
            $result->free_result();
            return $output;
        }

        public function GetActive()
        {
            $Key = $this->PrimaryKey;
            if($this->db->field_exists($this->RemovedKey, $this->TableName))
                $this->db->where(sprintf('(%1$s is null or %1$s = 0)', $this->RemovedKey));
            $result = $this->db->get($this->TableName);
            $output = array();
            if($result->num_rows() > 0)
            {
                foreach($result->result() as $row)
                    $output[$row->$Key] = $row;
            }
            $result->free_result();
            return $output;
        }

        public function Get()
        {
            $Key = $this->PrimaryKey;
            $result = $this->db->get($this->TableName);
            $output = array();
            if($result->num_rows() > 0)
            {
                foreach($result->result() as $row)
                    $output[$row->$Key] = $row;
            }
            $result->free_result();
            return $output;
        }

        public function Save($Row)
        {
            $Copy = clone $Row;
            $Fields = $this->db->field_data($this->TableName);
            foreach($Copy as $Key => $Value)
            {
                $found = false;
                foreach($Fields as $Field)
                {
                    if($Key == $Field->name)
                    {
                        $found = true;
                        break;
                    }
                }
                if(!$found) unset($Copy->$Key);
            }
            $X = new stdClass();
            foreach($Copy as $Key => $Value)
            {
                if(in_array($Key, $this->Reserved))
                    $NK = $this->escape($Key);
                else
                    $NK = $Key;
                $X->$NK = $Value;
            }
            $Copy = $X;
            unset($X);
            $PrimaryID = $this->PrimaryKey;
            if(isset($Row->$PrimaryID) && $Row->$PrimaryID !== null)
                $this->db->update($this->TableName, $Copy, array($this->PrimaryKey => $Copy->$PrimaryID));
            else
            {
                $this->db->insert($this->TableName, $Copy);
                $Row->$PrimaryID = $this->db->insert_id();
            }
            return $Row;
        }
    }
?>

Above is my BaseModel class. As you can see, you pass a table and primary key when creating it:

Code:

<?
    if (!defined('BASEPATH')) exit('No direct script access allowed');
    if(!class_exists('BaseModel')) require_once(APPPATH . 'libraries/Classes/BaseModel.class.php');

    class Address_model extends BaseModel
    {
        public function __construct() { parent::__construct('Address', 'AddressID'); }

        public function GetByUserID($UserID)
        {
            $this->db->where('UserID', $UserID);
            return $this->GetActive();
        }
    }
?>

This, like above allows me to expose Get, GetActive and GetSingle and if I need to create specific queries, I can simply create them in the actual model it self, and still reference the Get/GetActive/GetSingle afterwards.

In my above code, I have got around this issue by simply checking against my own list of reserved words and escaping those. I also have to TWO copies of the object being passed in, which is something i'm not happy about but it works.

I simply call Save on the model by passing it an object. If the object has been retrieved from the database, there will be a column as stated by the PrimaryKey.

If that PrimaryKey is present, it will update, otherwise it will insert.

Simples

Cheers

Gavin

[eluser]WanWizard[/eluser]
Explain this part for me:

Code:

foreach($Copy as $Key => $Value)
{
    if(in_array($Key, $this->Reserved))
        $NK = $this->escape($Key);
    else
        $NK = $Key;
    $X->$NK = $Value;
}

suppose you escape KEY, $NK becomes `KEY`, and then you create a class property $X->`KEY` to which you assign the value? And PHP likes that?

What I find interesting is that none if the property names of the $copy object are being escaped by the update method(). Which from a first glance at the code I can't explain it. The update() method passes you object to the set() method. That will convert the object to an array, and then loop though it to protect the keys and (optionally) the values, and add them to the ar_set array.

I suggest adding some debugging at various stages in this process, starting by the code above, to see where it goes wrong.

[eluser]gRoberts[/eluser]
Without the above piece of code, it will simply throw the error in my original post.

Even in the documentation, it doesn't escape the column names: http://ellislab.com/codeigniter/user-gui...ecord.html

PHP will allow you to create a property out of anything, as far as I am aware.

[eluser]WanWizard[/eluser]
Very strange.

If you go to our demo site, http://exitecms8.exitecms.org, and click open the profiler at the bottom, you'll see that all queries are properly escaped. And that's standard CI (1.7.2, but 2.0 behaves the same).

I just went through the code again, and I can't see how you would get passed escaping column names. No change made to the property _protect_identifiers? Not using an old version of CI? Or a modified core? No strange libraries overloading the database classes?

[eluser]gRoberts[/eluser]
Hi there,

I'm using a vanilla 1.7.2 setup, the only changes are the classes I pasted in my original post and a MY_router

Is there any configuration that enables this ?

Cheers

[eluser]WanWizard[/eluser]
Not that I'm aware of. Hence the "very strange"...

[eluser]gRoberts[/eluser]
It must be my setup as I have just uploaded a fresh copy of 1.7.2 downloaded today from CI and put the following code in (only changed the config/database files and added a controller)

Code:

$this->load->database();

            //create
            $cls = new stdClass();
            $cls->Key = 'class';
            $cls->SafeColumn = 'test';
            $this->db->insert('test', $cls);
            $cls->RowID = $this->db->insert_id();

            $arr = array();
            $arr['Key'] = 'array';
            $arr['SafeColumn'] = 'test';
            $this->db->insert('test', $arr);
            $arr['RowID'] = $this->db->insert_id();

            $cls->SafeColumn = 'changed';
            $this->db->update('test', $cls, array('RowID' => $cls->RowID));

            $arr['SafeColumn'] = 'changed';
            $this->db->update('test', $arr, array('RowID' => $arr['RowID']));

it worked :|

Thanks again

[eluser]Reneesh T K[/eluser]
I have an issue like codeigniter is not protecting the field names after calling a select statement with the second parameter set to 'FALSE'

I have resolved it by adding 2 lines of code in a function.

I had explained it in the below url.

http://myphplibrary.blogspot.in/2012/09/...-with.html