CodeIgniter Forums
DB_Forge, default value - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Development (https://forum.codeigniter.com/forumdisplay.php?fid=6)
+--- Forum: CodeIgniter 3.x (https://forum.codeigniter.com/forumdisplay.php?fid=17)
+--- Thread: DB_Forge, default value (/showthread.php?tid=607)

Pages: 1 2


DB_Forge, default value - danielsan - 12-26-2014

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


RE: DB_Forge, default value - jlp - 12-31-2014

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.


RE: DB_Forge, default value - mwhitney - 01-02-2015

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',
)); 



RE: DB_Forge, default value - danielsan - 01-03-2015

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.


RE: DB_Forge, default value - mwhitney - 01-05-2015

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


RE: DB_Forge, default value - harpreet - 03-31-2015

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()'
                                         




RE: DB_Forge, default value - CroNiX - 03-31-2015

(03-31-2015, 06:23 AM)harpreet Wrote: 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.


RE: DB_Forge, default value - harpreet - 03-31-2015

same table cannot have two timestamps fields.


RE: DB_Forge, default value - harpreet - 03-31-2015

agreed


RE: DB_Forge, default value - CroNiX - 03-31-2015

(03-31-2015, 07:09 AM)harpreet Wrote: 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.