The new drupal 6 schema api is missing the 'time' type for creating fields.
It's a serious problem.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Because date and time types are not well-standardized across databases, Drupal avoids using them. Core uses all unix timestamps. Are there any contribs that make use of an explicit datetime field in the database rather than a unix timestamp?

Gábor Hojtsy’s picture

Version: 6.x-dev » 7.x-dev
Priority: Critical » Normal

Not critical. Feature requests go against Drupal 7.

KarenS’s picture

Schema API adds a datetime, but not a time field. Core uses neither datetime nor time fields anywhere. The new version of the Event modules uses a datetime field and I was planning to add a datetime field to the Date module. I don't know of any contrib modules that use a time field.

alcroito’s picture

The Event module uses 'TIME' fields, not datetimes. I have to port it to d6, and because there is no time type, it would get difficult.

KarenS’s picture

I just checked again, because I was sure it uses datetime fields. The new Event module uses datetime fields for the event dates, but it uses time fields in the timezone database.

I would file an issue against the Event module about this. Killes (the Event module maintainer) needs to know that the D6 Schema API won't support his database structure and be involved in the decision about what to do about that.

It is not *required* to use Schema API to define the database, you can still do it the way it's done in D5, by writing out the necessary SQL statements in the install file. It's just easier to use Schema API.

beginner’s picture

Title: Schema API lacks the 'time' type » Schema API lacks the 'time' and 'date' type
Anonymous’s picture

My preference is TIMESTAMP when I need a historical stamp, to use TIMESTAMP with a time of 00:00:00 for a date field (sorry, I should have said column ;) and I would use TIME for time of day fields columns. I always store DATE/TIME data in UTF or GMT representation and covert it to user local timezone for display. Also the stored DATE/TIME order is always CYMD/HMS so that it sorts correctly.

Anonymous’s picture

As for TIME; a time only value can be stored as an INT, NUMERIC or VARCHAR field. I think my preference (really, undecided) would be to use a NUMERIC. I do think that TIME should be one of the supported data types for the Schema API though.

alexanderpas’s picture

may I point everyone towards:

ISO8601: Numeric representation of Dates and Time

Wikipedia: http://en.wikipedia.org/wiki/ISO_8601
Summary from ISO: http://www.iso.org/iso/support/faqs/faqs_widely_used_standards/widely_us...
Full document (zip-pdf, 228KB): http://isotc.iso.org/livelink/livelink/4021199/ISO_8601_2004_E.zip?func=...

how about a varchar field that simply stores the ISO date and/or time

grub3’s picture

>how about a varchar field that simply stores the ISO date and/or time

varchar is not a good solution to store time, because you may search by time or ORDER BY time,
this could result in a cast during execution plan and produce sequential access on data.
On large database, it can produce a real overead, especialy in JOINs.

It is better to choose the native time/date format in databases and adapt PHP code accordingly.
The best would be to use SQL99 standards and adapt non-standard databases accordingly on PHP side (using an int).

Kind regards,
JMP

alexanderpas’s picture

> varchar is not a good solution to store time, because you may search by time or ORDER BY time,
> this could result in a cast during execution plan and produce sequential access on data.
> On large database, it can produce a real overead, especialy in JOINs.

uhm... could you explain this more... i don't think i understand this..

but remember that ISO 8601 field (date, time and also combined) are lexicon sortable.
also use only one ISO 8601 field, as it can store date, time, date&time, duration with combinations, all in the same field. (no seperate date, time and/or duration fields)

this'll make the ISO 8601 perfect for storage of (log and actual)events etc. as it'll record all information together.

> It is better to choose the native time/date format in databases and adapt PHP code accordingly.

Not from a data-portability view, adapting to different standards for different databases, will make switching databases increasingly difficult (vendor lock-in)

damiancugley’s picture

ISO 8601 and RFC 3339 datetime representations are only sortable if they are all in the same time zone. Consider 2007-01-01T01:00:00+01:00 versus 2006-12-31T23:00-08:00.

You could add a warning to the documentation for the Schema API that datetimes are not the usual way to represent datetime values. Being new to the PHP world, I wasted a day trying to use datetimes and then giving up and switching to UNIX timestamps because there is no sane way to get them to and from the database and to and from form widgets anyway.

alexanderpas’s picture

yes, unless you convert to UTC before storage... (okay, i know that's agains the "do not tinker with data" principle, but) this way, you can always show the time in the timezone of the user!

Anonymous’s picture

I agree that date and time should be stored as UTC/GMT time. Then display adjustments can simply apply the timezone for the user. It is a rule with which I see many vendors of application systems following. However, if one decides to do otherwise the timezone data must also be stored to fully represent the time value.

hswong3i’s picture

Maybe we should give a look to http://phplens.com/lens/adodb/tips_portable_sql.htm. ADOdb already did these data/time abstraction for many years, which work fine with many DB backend.

I would even hope to include this with http://drupal.org/node/167335 for better abstraction. The main point is: if we really need these functionality? If so, let's work it out :)

jefftrnr’s picture

I've found this bit of useful php code for building a time-select options array. It's useful in telling Drupal how to build a select for a varchar text field - with options set to a range of times, spaced by whatever you prefer. Just paste these bits of code (with param adjustments) into a custom module and into your Allowed values list -->"PHP Code" box. It's at the bottom of the Field configure page (D6).

Taken from http://codingforums.com/archive/index.php?t-87784.html

Paste this into a custom module file...


/**
* create_time_range
*
* @param mixed $start start time, e.g., 9:30am or 9:30
* @param mixed $end end time, e.g., 5:30 or 17:30
* @param string $by 1 hour, 1 mins, 1 secs, etc.
* @access public
* @return void
*/
function create_time_range($start, $end, $by='30 mins') {

$start_time = strtotime($start);
$end_time = strtotime($end);

$times = array();
for ( ;$start_time < $end_time; ) {
$times[] = $start_time;
$start_time = strtotime('+'.$by, $start_time);
}
$times[] = $start_time;
return $times;
}

And Paste this into the CCK Php Code box:

// enter time range and increment here
$times = create_time_range('9:00am', '11:00pm', '30 mins');
// format the unix timestamps
foreach ($times as $key => $time) {
$times[$key] = date('g:i a', $time);
}
return $times;
sumgai’s picture

One of the reasons I use MySQL is because of its support for field types I need in my applications (some use spatial extensions in addition to date/time/etc. features). The lack of some way to handle these field types in the Drupal Schema system makes the Schema system unusable for some of my modules.

Although I appreciate the desire to offer some type of "standard" database interface, I would still like to be able to accommodate database-specific field types in the Drupal Schema system so I can continue to use it for its other benefits (I love being able to access a schema in my modules, etc.).

Couldn't there be some extension made to the schema system so "user-defined" field types could be supported?

If you want to require submitted modules to be database independent, that's fine, but "private" modules would benefit more from the schema system if something like this were supported.

You could have a schema field type that looked like:

array(
'type' => 'user',
'value' => 'xxx',
...(any of the other standard attributes could go here)...
)

Then, module developers could put Date, Timestamp, Point or whatever else they needed into the 'xxx'.

The schema would just pass that into the SQL generated in the same place as it would place any 'standard' field type.

You could also add some type of schema 'requirement' field that said that this schema is only compatible with database x.

Thanks for your consideration of this feature request.

Crell’s picture

Schema API is hopefully going to get some much needed improvements now that the main DBTNG patch is in. I'm still working out exactly how that will work. Definitely #298669: Add query logging per Connection is a prerequisite, as it will allow us to get rid of the hack that is update_sql() and make the Schema API richer. Then we can see about things like database-specific types.

sumgai’s picture

As long as we're at it, it would be nice to have a db_num_rows().

I don't know about other DB's, but MySQL doesn't return the # rows for a SELECT if you use mysql_affected_rows(). It only returns the right answer for mysql_num_rows().

The function could return an error if it's a function that's not supported by the database being used by Drupal.

sumgai’s picture

I would also suggest some schema field that specifies whether a particular database type is required (to go along with the database-specific or user-generated) datatypes.

sumgai’s picture

This isn't the right place for this, but just out of curiosity, does db_affected_rows return a real number for a SELECT query in non-MySQL databases?

Thanks,

m

Damien Tournoud’s picture

@sumgai: db_affected_rows() is only useful for UPDATE and DELETE queries. For SELECT queries, the right way is to use COUNT().

Crell’s picture

db_affected_rows() is deprecated anyway and will be removed before Drupal 7 is released.

Please, let's stay on topic here.

mikl’s picture

Okay, this is one of my particular pet peeves. Both when looking at the documentation (http://dev.mysql.com/doc/refman/5.0/en/time.html and http://www.postgresql.org/docs/8.3/static/datatype-datetime.html#AEN4806 plus http://www.postgresql.org/docs/8.3/static/datatype-datetime.html#AEN4735 and http://dev.mysql.com/doc/refman/5.0/en/datetime.html ) and in my practical testing, there are no compatibility issues when you stick to ISO 8601 (as any programmer should).

Of course, if you start using database-specific SQL-functions, you are in trouble, but that is not specific to these particular data types.

I see no reason not to support these datatypes. In fact, I consider it a bug in Drupal 6 that we don't. I know that it's probably not going to be changed in D6, so I'm not going to change the version of this issue.

Without looking at Drupal as it has been or what we normally do with dates and times, DATE, TIME, and DATETIME are all perfectly good and valid RDBMS data types and there are real use cases for all of them (Compatibility with other systems, wanting to have time information without date or vice versa).

We need not use them in core. There might be long debates about how it's better to use UNIX timestamps and integer/string-mangling to replicate these column types instead of just using them outright.

But as one who spends most of his time writing Drupal modules, I'd just like to be able to be able to design my database tables as I want to without having opinionated tools that require silly hacks like these:

$columns[$abbr . '_start'] = array(
  'type' => 'varchar',
  'pgsql_type' => 'time',
  'mysql_type' => 'time',
);

So what's to be done? A few extra lines added to db_type_map should be enough in D6, but I don't if the picture has changed with DBTNG…

mikl’s picture

Hmm, it seems to be pretty much the same with regards to the type map, now moved to a class method on DatabaseSchema::getFieldTypeMap but in essence pretty much the same.

I don't have much insight into the query builder, but on first look it doesn't seem that there's any problems there.

seanburlington’s picture

Status: Active » Needs review
FileSize
1.48 KB

Hello

coming from over here http://drupal.org/node/293483

patch for date and time types

One problem with using ints to store dates is the limits this imposes
"Being 32 bits (of which one bit is the sign bit) means that it covers a range of about 136 years in total. The minimum representable time is 1901-12-13, and the maximum representable time is 2038-01-19. At 03:14:07 UTC 2038-01-19 this representation overflows."
http://en.wikipedia.org/wiki/Unix_time

I guess you can use datetime fields to store dates - but I don't see the benefit.

I have in the past provided search options for events that happen on a given day of the week - easy to do with date functions - but otherwise you'd have to load the whole table to PHP.

What prompted me to patch this was that the schema contrib module was showing errors after I created a date field, I want to use the schema module to check the db schema is correct - but I can only do this if it recognises the data types I use.

As a framework - where Drupal competes with Symfony, Zend, CodeIgniter, Rails, CakePHP etc - this sort of flexibility is required.

NB this patch is untested at the moment. I'll try and find time to write some tests for it.

seanburlington’s picture

This patch has the same functional code change as before

I've added unit tests

These tests pass for me on MySQL 5.0.67-0ubuntu6 and postgresql 8.3.7-0ubuntu8.10.1

I would have tested on sqlite - but I couldn't see how to configure it - the config screen still shows options for username/password/host - but not path/file

Damien Tournoud’s picture

Status: Needs review » Needs work

One problem with using ints to store dates is the limits this imposes
"Being 32 bits (of which one bit is the sign bit) means that it covers a range of about 136 years in total. The minimum representable time is 1901-12-13, and the maximum representable time is 2038-01-19. At 03:14:07 UTC 2038-01-19 this representation overflows." http://en.wikipedia.org/wiki/Unix_time

I guess you can use datetime fields to store dates - but I don't see the benefit.

You can use 64 bits integers, those are supported on all databases, and they cover about 600 billion years.

I have in the past provided search options for events that happen on a given day of the week - easy to do with date functions - but otherwise you'd have to load the whole table to PHP.

Easy to do, but:

  • You would need a portable function to do that across databases. That function doesn't exist. For example, MySQL has a weekday() function (0 = Monday, 1 = Tuesday, ... 6 = Sunday), while PostgreSQL has a dow() (0 = Sunday, ... 6 = Saturday). SQLite has no date functions at all.
  • You would need to add a special "day of the week" column if you want to do any kind of efficient look-up for that. This alone completely defeat the purpose of having a special type.

A lot could be done to improve date/time management in Drupal database layer. The first step for this is to define a common set of helper functions and a throughout set of tests to verify that those functions are consistent across database engines.

Before we do that, I'm against adding the DATE type. In the current state of things, it is a false good idea, as it would only promote bad, non-portable practices.

seanburlington’s picture

Status: Needs work » Needs review

Compatibility issues already exist.

They are there with datetime - and in fact with all data types - you can even use date functions from ints (eg FROM_UNIXTIME)

Date/time types can be used in a completely cross-vendor way.

And as has been pointed out this limitation doesn't prevent people writing date fields, it just pushes them away from the schema API if they choose to do so.

But perhaps more importantly for me:

'Although Drupal is often described as a "content management system" (CMS) it is also a "content management framework" (CMF). '
http://drupal.org/getting-started/before/overview

This is how I've mostly used Drupal; working for (generally large) companies who need a quick start to their project without compromising on features - and they need the ability to integrate with existing data.

Drupal isn't always working alone, and isn't always first to have a say in data specification.

I want the ability to choose data types - especially on custom code that will only ever run on one database.

Encouraging cross-vendor SQL in public code is surely a separate issue from deciding on base level functionality.

Damien Tournoud’s picture

Status: Needs review » Needs work

Encouraging cross-vendor SQL in public code is surely a separate issue from deciding on base level functionality.

Not at all. Drupal is not a lazy framework that "pretends" to be compatible with 30 database engines. Drupal is proven to run out of the box on MySQL, PostgreSQL and SQLite. That's unique, and it has been a sizable effort.

You are taking the problem the wrong way: first define a set of base level functionality that should be available for all our supported database engines. Then implement it.

We cannot pretend to support DATE/TIME types until we have (1) defined a set of portable functions and operators that manipulate those types and (2) written a set of tests that allow us to be confident that those work across the databases we are supporting.

Crell’s picture

We don't need to support everything there is about date/time types. We just need to provide a common base. Date module manages this now, or close enough to support postgres and mysql. I believe it routes most things through a DB-level date_format(). If we can at least do that much, we can move core over to native times which buys us a great deal of flexibility.

Talk to KarenS for the details. :-)

alexanderpas’s picture

and again, i wish to point to ISO8601
which will cover any time value between
00000101T000000Z and 99991231T240000Z (read: january 1st year 1 BC(year 0) 00:00:00 and december 31 year 9999 24:00:00 (january 1st 10000 00:00)) with a fixed string length (after normalisation) in it's default writing.
and any time, with any precision, with it's extended writing.
example: +140950821T132345164735Z (read: august 21st year 14095 13:23:45 and 164 miliseconds and 735 microseconds)

so there will never be any epoch/boudary issues (which you still have with 64bit (unsigned) int [1]) compatibility issues (leap seconds anyone[2]) or overflow issues (not considered a problem in default writing[3], and non-existing in expanded format[4])

read the wikipedia article on ISO 8601 to see all possibilities!!! (and don't forget, parsing =! storing!)

KarenS’s picture

This can easily turn into another bike shed issue. There have been countless issues about whether or not we 'need' real date fields in the database.

However, integer representations of unix timestamp values are not dates, they lack information about timezones and there are no native database functions that can be used to do things like extract a month or a day from the value. You have to cast unix timestamp values as dates (in different ways in every database) and then use database-specific methods of extracting date values from those dates. Or you have to retrieve all your values, do your conversion in PHP, and then figure out which ones you want.

ISO dates are not anything that the database understands as 'dates' either, and I know of no easy ways to parse them automatically in SQL.

People who don't need to parse the information are perfectly happy with timestamps, and ISO dates are great for storing 'fuzzy' dates that might have any kind of granularity, but those of us who are trying to do things like create calendars, charts, and custom Views of dates using SQL queries can do it much better if we use data that is already in the right native database format. And with a good set of cross-database functions (which the Date API has been trying to refine for the last couple of years) we can do a pretty good job of easily creating complex queries to do things like "retrieve dates greater than midnight today and less than midnight one week from now, grouped by day and hour", far more efficiently than we could do anything comparable in PHP.

I definitely think we need to support native date, time, and datetime fields in Schema API. Making everyone who needs that kind of functionality create their own custom solutions makes no sense, a basic framework for handling dates in intelligent ways should be supported by core.

Damien Tournoud’s picture

Back to the original plan (outlined in #31):

(1) define a set of portable functions and operators that manipulate those types
(2) write a set of tests that allow us to be confident that those work across the databases we are supporting

By the way, I think I miss the point of executing queries like "retrieve dates greater than midnight today and less than midnight one week from now, grouped by day and hour" using native date/time types and functions. Can MySQL really use indexes on a query like that? If it can't (I doubt it does), you would have to create yourself derived columns (in that case a day column and an hour column), and make the database engine index that. Once you have done that, native support of date/time types has zero added value, as you are basically using those for storage only.

Crell’s picture

Even if the SQL DB can't fully index that query, it's still happening in C code and then returning to the PHP code only the records you want. There are many cases in which that will still be faster than retrieving too much data from SQL and then trying to filter it in PHP (as well as cases where the opposite is true; that will vary case by case).

I'd say the base minimum would be "store in DB native format" and "convert from DB native format to some standard PHP-side format" (eg, DateTime object, ISO date string, whatever.) If we can do that cleanly, that's on par with what we have now with timestamps (the native and PHP-side formats just being ints in that case). Anything else on top of that is gravy.

mikl’s picture

#35: I think that database abstraction/portability should be a different issue. To me it is, like Crell says, just gravy. While it would be nice to have, what this issue is about is (at least for me) that Drupal is getting in the way of my (as a module developer) moderately well-informed decisions about what database column types to use.

killes@www.drop.org’s picture

Status: Needs work » Needs review

It seems that there's a fully functional patch here.

Can we have the bikeshed discussion /after/ it got applied?

moshe weitzman’s picture

I really defer to KarenS here. She has been supporting a massive user base with date module for a long long time. She states that schema should support these types.

Lets not let perfection impede progress. We should proceed with this and improve from there. My .02.

Damien Tournoud’s picture

I don't believe that the Date module is a great role model on how to do things properly here. Example of a condition generated by Date (via Views):

WHERE DATE_FORMAT(ADDTIME(FROM_UNIXTIME(node.created), SEC_TO_TIME(7200)), '%Y-%m-%%d\T%H:%i:%%s') >= '2009-06-22T22:21:58' AND DATE_FORMAT(ADDTIME(FROM_UNIXTIME(node.created), SEC_TO_TIME(7200)), '%Y-%m-%%d\T%H:%i:%%s') <= '2009-06-29T22:21:58'

This makes no sense at all.

mfb’s picture

Status: Needs review » Needs work

There's a notice Undefined index: time:normal when testing in sqlite. Presumably this is caused by typo 'time:nornaml'

seanburlington’s picture

Happy to fix this - if someone can let me know how to configure an SQLite database via the install page

(I have SQLite installed - but host/username/password don't make much sense - and where do you put the file?)

mfb’s picture

@seanburlington see INSTALL.sqlite.txt (disclaimer I haven't looked at it in a while.. hope it works for you)

seanburlington’s picture

Status: Needs work » Needs review
FileSize
5.29 KB

Thanks - I guess some of those notes will be included onscreen by release date.

I've corrected the typo and run the tests on an SQLite install - works for me.

Status: Needs review » Needs work

The last submitted patch failed testing.

mfb’s picture

Status: Needs work » Needs review

retesting

Crell’s picture

Status: Needs review » Reviewed & tested by the community

I like simple.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

This solves a critical DX problem and I support its inclusion. There are some coding standards violations though (detailed below).

Question. Does it make sense (not in this patch) to support something like hook_schema_api_types_alter() so that contrib modules can add support for other column types such as enum or longitude/latitude (supported by pgsql)? Seems like currently folks in that bucket have to either hack core or write SQL statements by hand.

+++ includes/database/pgsql/schema.inc
@@ -242,7 +242,10 @@ class DatabaseSchema_pgsql extends DatabaseSchema {
+      'date:normal' => 'date',
+
       'datetime:normal' => 'timestamp without time zone',
+      'time:normal'     => 'time without time zone',

I don't know why the other three were spaced out with double-lines between them, but this one should match.

+++ modules/simpletest/tests/database_test.test
@@ -2749,3 +2749,109 @@ class DatabaseTransactionTestCase extends DatabaseTestCase {
+
+/**
+ * Test proposed new data types for the schema API
+ * 
+ */
...
+  /**
+   * Test the date data type
+   *
+   */

Comments should end in a period. Also, there's an extra blank line after the description that doesn't need to be there.

+  public static function getInfo() {
+    return array(
+      'name' => t('Extra Types tests'),
+      'description' => t('Test the Extra Types.'),
+      'group' => t('Database'),
+    );
+  }

No t()s around stuff in getInfo().

+       $date_table = array('fields' => array(
+         'date_field' => array(
+           'description' => t('Test Date field'),
+           'type' => 'date',
+           'not null' => FALSE,
+	     ) 
+         )
+       );
...
+       $time_table = array('fields' => array(
+         'time_field' => array(
+           'description' => t('Test Time field'),
+           'type' => 'time',
+           'not null' => FALSE,
+	     )
+         )
+       );

Identation off here. Please move 'fields' to its own line and indent accordingly.

Also, ) should be ), everywhere (apart from the final ) that closes the array).

+       $this->assertEqual($ret[0]['success'], 1, t('created table with date field'));
...
+       $this->assertEqual($ret[0]['success'], 1, t('created table with time field'));
# and elsewhere...

The assertion string should begin with a capital letter.

...
+       $date = $res->fetch()->date_field;
+       $this->assertEqual($date, '1856-12-31', t('date retrieved in order ') . $date);
+       $date = $res->fetch()->date_field;
+       $this->assertEqual($date, '2001-01-01', t('date retrieved in order ') . $date);
+       $date = $res->fetch()->date_field;
+       $this->assertEqual($date, '2100-06-30', t('date retrieved in order ') . $date);
...
+       $time = $res->fetch()->time_field;
+       $this->assertEqual($time, '00:01:00', t('time retrieved in order ' . $time));
+       $time = $res->fetch()->time_field;
+       $this->assertEqual($time, '12:59:00', t('time retrieved in order ' . $time));
+       $time = $res->fetch()->time_field;
+       $this->assertEqual($time, '23:17:00', t('time retrieved in order ' . $time));

Please use proper placeholders. @time.

+       $res = db_query("select date_field from {date_table} order by date_field");
...
+       $res = db_query("select time_field from {time_table} order by time_field");

No need to double-quote this if there are no variables.

kjy07’s picture

FileSize
6.32 KB

Rerolled per webchick.

kjy07’s picture

FileSize
6.33 KB

D'oh! Re(re)-rolled. :)

kjy07’s picture

Status: Needs work » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs documentation, +DrupalWTF

Thanks, kjy07!

Committed to HEAD with a couple other minor touch-ups.

This needs to be documented in the module upgrade guide: http://drupal.org/update/modules/6/7

Crell’s picture

Status: Needs work » Fixed

It has been documented.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

jeffschuler’s picture

To help alleviate anyone else's confusion looking for this code, this change was rolled back in #866340: Remove support for date and time types.