DB_Forge, default value

Hi,

I hope I've put this discussion in the right forum since it's, in the end, a feature discussion.

I'm using DBForge to create my database. I would like the following SQL emitted from DBForge:

Code:

CREATE TABLE `UserEvents` (
 `id` int(9) NOT NULL AUTO_INCREMENT,
 `Ts` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP
);

I do:

Code:

$this->dbforge->add_field(
           array(
                 'Ts' => array(
                     'type' => 'TIMESTAMP',
                     'default' => 'CURRENT_TIMESTAMP',
                 ),
             )
         );

However this emits (note the quotes around CURRENT_TIMESTAMP):

Code:

 `Ts` timestamp NOT NULL DEFAULT 'CURRENT_TIMESTAMP'

The reason is found in system/database/DB_forge.php, function _attr_default, where $this->db->escape() is used on the supplied default value. While this would be a reasonable way in most cases, in this case it's not since the default value should not be the string 'CURRENT_TIMESTAMP' but instead a MySQL function CURRENT_TIMESTAMP.

I would suggest adding another attribute (e.g. DEFAULT_LITTERAL) which is not run through $this->db->escape()
. These would be mutually exclusive (if both are present). In fact, I've already implemented this in my own project and would like to hear any comments and also see if there is any interest in adding this to the default codebase.

My humble patch:

Code:

--- DB_forge.php.orig   2014-12-17 22:01:42.337112070 +0100
+++ DB_forge.php        2014-12-17 22:07:55.365566945 +0100
@@ -940,18 +940,28 @@

               if (array_key_exists('DEFAULT', $attributes))
               {
-                       if ($attributes['DEFAULT'] === NULL)
-                       {
-                               $field['default'] = empty($this->_null) ? '' : $this->_default.$this->_null;
+                       $arraykey = 'DEFAULT';
+               }
+               elseif (array_key_exists('DEFAULT_LITTERAL', $attributes))
+               {
+                       $arraykey = 'DEFAULT_LITTERAL';
+               }
+               else
+               {
+                       return;
+               }

-                               // Override the NULL attribute if that's our default
-                               $attributes['NULL'] = TRUE;
-                               $field['null'] = empty($this->_null) ? '' : ' '.$this->_null;
-                       }
-                       else
-                       {
-                               $field['default'] = $this->_default.$this->db->escape($attributes['DEFAULT']);
-                       }
+               if ($attributes[$arraykey] === NULL)
+               {
+                       $field['default'] = empty($this->_null) ? '' : $this->_default.$this->_null;
+
+                       // Override the NULL attribute if that's our default
+                       $attributes['NULL'] = TRUE;
+                       $field['null'] = empty($this->_null) ? '' : ' '.$this->_null;
+               }
+               else
+               {
+                       $field['default'] = ($arraykey == 'DEFAULT' ? $this->_default.$this->db->escape($attributes[$arraykey]) : $this->_default.$attributes[$arraykey]);
               }
       }

Kind regards

Daniel Sahlberg

Why not raise a PR with this change, and see what the comments are?
I don't know if it is MYSQL-specific, or more generally useful.
The latter would make it a better candidate for an enhancement.

James Parry
Project Lead

Since almost anything which would require this functionality would be database-specific (and CURRENT_TIMESTAMP is certainly MySQL-specific), you're probably better off just defining the field as

PHP Code:

'Ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP', 


For example:

PHP Code:

$this->dbforge->add_field(array(
    'id' => array(
        'type' => 'int',
        'constraint' => 9,
        'null' => false,
        'auto_increment' => true,
    ),
    'Ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP',
)); 


Bonfire

Practical CodeIgniter 3
CodeIgniter Testing Guide

Thank you for your answer. I wasn't aware of the possibility to mix the two syntaxes.

At this time I agree with you, since it's probably db-specific, it's reasonable to do it this way. Anyway, the code is available in the forum if anyone would need it in the future.

I wouldn't have been aware of it either, but I've run into this exact problem before:
https://github.com/ci-bonfire/Bonfire/co...8cfe12da18

Bonfire

Practical CodeIgniter 3
CodeIgniter Testing Guide

really helpful thank you,
This is how i am using them with two fields

PHP Code:

'created_at  timestamp default current_timestamp',
            'updated_at' => array(
                                         'type' => 'varchar',
                                         'constraint' => 250,
                                         'null' => true,
                                         'on update' => 'NOW()'
                                         ) 


Harpreet Bhatia
Web Developer / Analyst
Technical Reviewer - "Codeigniter 2 Cookbook"

really helpful thank you,
This is how i am using them with two fields

PHP Code:

'created_at  timestamp default current_timestamp',
            'updated_at' => array(
                                         'type' => 'varchar',
                                         'constraint' => 250,
                                         'null' => true,
                                         'on update' => 'NOW()'
                                         ) 


Except you don't want to use a varchar type on a timestamp field. You loose all database specific time manipulation functions. Better to use a 'datetime' field there.

same table cannot have two timestamps fields.

Harpreet Bhatia
Web Developer / Analyst
Technical Reviewer - "Codeigniter 2 Cookbook"

agreed

Harpreet Bhatia
Web Developer / Analyst
Technical Reviewer - "Codeigniter 2 Cookbook"

same table cannot have two timestamps fields.

Sure it can. You can have as many timestamp or other "date/time" type related columns as you want. What makes you say you can't? I just created a table with 3 "timestamp" and 3 "datetime" fields with no problem.