[development] Extend database abstraction layer, to include table creation.

Adrian Rossouw adrian at bryght.com
Sat May 13 05:00:34 UTC 2006


On 12 May 2006, at 6:00 PM, Dries Buytaert wrote:
> Can't we move forward without these functions?  It looks like we  
> are introducing more and more dependencies to that makes it harder  
> to move the install system forward.
I never said it was a dependency. I said it would make it simpler. We  
can still do it without this, it's just much more work, the same with  
every single other schema we are
supporting in duplicate or even triplicate.

> While an additional database layer (SQL is already an abstraction  
> layer) makes it easier to port Drupal to other database systems, it  
> makes it harder to understand what is going on.  It is a lot less  
> accessible to people that are new to Drupal, but that do have a SQL  
> background.  So personally, I'm not all that convinced that these  
> functions are a good idea ...
This is not about supporting more db types, this is about reducing  
code, and reducing duplication of it.

I don't think it's sane that every single change to the schema needs  
to happen in 4 places.

I don't like the fact that a contrib module accepting a patch to  
support postgres, could hold up development of their module.

I don't like the fact that postgres and mysql schemas might get out  
of synch in contrib modules (WITHIN THE SAME _install() FUNCTION).
Something like that is incredibly annoying to fix, and increases the  
amount of code developers need to write, some of which they might
not be capable of writing (ie: the postgres schema updates).

> I, for one, find it much easier to read an SQL statement than some  
> (new) Drupal-specific table definition.  I don't want to learn or  
> use a such definition language.  Such functions make the database  
> scheme less transparent.
mysqldump -usomething database > database.pgsql
or
pgsql -Usomething database > database.pgsql

These proposed functions are essentially theme_ functions with a  
db_query on the result at the end.
The 'type' is essentially the index for a map of the types each  
database uses to represent what we need.

The amount of code we stand to remove :
(apart from the ~1800 lines of sql we can remove with the extra schemas)

And an example of only one of the update_ functions we could have  
improved.
These functions seem to be an average of 20 lines of code, per  
function, per update
that does stuff like this, over ALL OF CORE AND CONTRIB.

---
The original (55 lines long)
---

function system_update_166() {
   $ret = array();

   $ret[] = update_sql("DROP TABLE {directory}");
   switch ($GLOBALS['db_type']) {
     case 'mysqli':
     case 'mysql':
       $ret[] = update_sql("CREATE TABLE {client} (
         cid int(10) unsigned NOT NULL auto_increment,
         link varchar(255) NOT NULL default '',
         name varchar(128) NOT NULL default '',
         mail varchar(128) NOT NULL default '',
         slogan longtext NOT NULL,
         mission longtext NOT NULL,
         users int(10) NOT NULL default '0',
         nodes int(10) NOT NULL default '0',
         version varchar(35) NOT NULL default'',
         created int(11) NOT NULL default '0',
         changed int(11) NOT NULL default '0',
         PRIMARY KEY (cid)
       )");
       $ret[] = update_sql("CREATE TABLE {client_system} (
         cid int(10) NOT NULL default '0',
         name varchar(255) NOT NULL default '',
         type varchar(255) NOT NULL default '',
         PRIMARY KEY (cid,name)
       )");
       break;

     case 'pgsql':
       $ret[] = update_sql("CREATE TABLE {client} (
         cid SERIAL,
         link varchar(255) NOT NULL default '',
         name varchar(128) NOT NULL default '',
         mail varchar(128) NOT NULL default '',
         slogan text NOT NULL default '',
         mission text NOT NULL default '',
         users integer NOT NULL default '0',
         nodes integer NOT NULL default '0',
         version varchar(35) NOT NULL default'',
         created integer NOT NULL default '0',
         changed integer NOT NULL default '0',
         PRIMARY KEY (cid)
       )");
       $ret[] = update_sql("CREATE TABLE {client_system} (
         cid integer NOT NULL,
         name varchar(255) NOT NULL default '',
         type varchar(255) NOT NULL default '',
         PRIMARY KEY (cid,name)
       )");
       break;
   }

   return $ret;
}

---
the new version (28 lines)
---

function system_update_166() {
   $ret = array();

   db_drop_table("directory");
   db_create_table(client, array(
         'cid' => array('type' => sequence),
         'link' => array('type' => 'short text', 'NOT NULL',  
'default' => ''),
         'name' => array('type' = 'tiny text', 'NOT NULL', 'default'  
=> ''),
         'mail' => array('type' = 'tiny text', 'NOT NULL', 'default'  
=> ''),
         'slogan' => array('type' = 'tiny text', 'NOT NULL',  
'default' => ''),
         'mission' => array('type' = 'tiny text', 'NOT NULL',  
'default' => ''),
         'users' => array('type' = 'int', 'NOT NULL', 'default' => 0),
         'node' => array('type' = 'int', 'NOT NULL', 'default' => 0),
         'version' => array('type' = 'tiny text', 'NOT NULL',  
'default' => ''),
         'created' => array('type' = 'timestamp' , 'NOT NULL' =>  
TRUE, 'default' => 0),
         'changed' => array('type' = 'timestamp' , 'NOT NULL' =>  
TRUE, 'default' => 0),
       ));

   db_create_table('client_system', array(
         'cid' => array('type' =>  'int', 'NOT NULL' => TRUE,  
'PRIMARY KEY' => TRUE),
         'name' => array('type' => 'short text', 'NOT NULL',  
'default' => '', , 'PRIMARY KEY' => TRUE),
         'type' => array('type' => 'short text', 'NOT NULL',  
'default' => ''),
       ));
       break;


   return $ret;
}

--
Adrian Rossouw
Drupal developer and Bryght Guy
http://drupal.org | http://bryght.com




More information about the development mailing list