Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The new drupal 6 schema api is missing the 'time' type for creating fields.
It's a serious problem.
Comment | File | Size | Author |
---|---|---|---|
#50 | 200953.patch | 6.33 KB | kjy07 |
#49 | 200953.patch | 6.32 KB | kjy07 |
#44 | schema_add_date_and_time_with_unit_tests.diff | 5.29 KB | seanburlington |
#28 | schema_add_date_and_time_with_unit_tests.diff | 5.29 KB | seanburlington |
#27 | add_date_and_time_to_schema_api.diff | 1.48 KB | seanburlington |
Comments
Comment #1
Crell CreditAttribution: Crell commentedBecause 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?
Comment #2
Gábor HojtsyNot critical. Feature requests go against Drupal 7.
Comment #3
KarenS CreditAttribution: KarenS commentedSchema 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.
Comment #4
alcroito CreditAttribution: alcroito commentedThe 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.
Comment #5
KarenS CreditAttribution: KarenS commentedI 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.
Comment #6
beginner CreditAttribution: beginner commented#252916: Schema doesn't support DATE nor TIME type is a duplicate of this one.
http://en.wikibooks.org/wiki/SQL_dialects_reference/Data_structure_defin...
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedMy 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 dayfieldscolumns. 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.Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedAs 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.
Comment #9
alexanderpas CreditAttribution: alexanderpas commentedmay 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
Comment #10
grub3 CreditAttribution: grub3 commented>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
Comment #11
alexanderpas CreditAttribution: alexanderpas commented> 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)
Comment #12
damiancugley CreditAttribution: damiancugley commentedISO 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.
Comment #13
alexanderpas CreditAttribution: alexanderpas commentedyes, 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!
Comment #14
Anonymous (not verified) CreditAttribution: Anonymous commentedI 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.
Comment #15
hswong3i CreditAttribution: hswong3i commentedMaybe 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 :)
Comment #16
jefftrnr CreditAttribution: jefftrnr commentedI'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...
And Paste this into the CCK Php Code box:
Comment #17
sumgai CreditAttribution: sumgai commentedOne 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.
Comment #18
Crell CreditAttribution: Crell commentedSchema 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.
Comment #19
sumgai CreditAttribution: sumgai commentedAs 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.
Comment #20
sumgai CreditAttribution: sumgai commentedI 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.
Comment #22
sumgai CreditAttribution: sumgai commentedThis 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
Comment #23
Damien Tournoud CreditAttribution: Damien Tournoud commented@sumgai: db_affected_rows() is only useful for UPDATE and DELETE queries. For SELECT queries, the right way is to use COUNT().
Comment #24
Crell CreditAttribution: Crell commenteddb_affected_rows() is deprecated anyway and will be removed before Drupal 7 is released.
Please, let's stay on topic here.
Comment #25
mikl CreditAttribution: mikl commentedOkay, 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:
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…
Comment #26
mikl CreditAttribution: mikl commentedHmm, 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.
Comment #27
seanburlington CreditAttribution: seanburlington commentedHello
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.
Comment #28
seanburlington CreditAttribution: seanburlington commentedThis 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
Comment #29
Damien Tournoud CreditAttribution: Damien Tournoud commentedYou can use 64 bits integers, those are supported on all databases, and they cover about 600 billion years.
Easy to do, but:
weekday()
function (0 = Monday, 1 = Tuesday, ... 6 = Sunday), while PostgreSQL has adow()
(0 = Sunday, ... 6 = Saturday). SQLite has no date functions at all.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.
Comment #30
seanburlington CreditAttribution: seanburlington commentedCompatibility 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.
Comment #31
Damien Tournoud CreditAttribution: Damien Tournoud commentedNot 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.
Comment #32
Crell CreditAttribution: Crell commentedWe 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. :-)
Comment #33
alexanderpas CreditAttribution: alexanderpas commentedand 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!)
Comment #34
KarenS CreditAttribution: KarenS commentedThis 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.
Comment #35
Damien Tournoud CreditAttribution: Damien Tournoud commentedBack 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.
Comment #36
Crell CreditAttribution: Crell commentedEven 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.
Comment #37
mikl CreditAttribution: mikl commented#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.
Comment #38
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedIt seems that there's a fully functional patch here.
Can we have the bikeshed discussion /after/ it got applied?
Comment #39
moshe weitzman CreditAttribution: moshe weitzman commentedI 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.
Comment #40
Damien Tournoud CreditAttribution: Damien Tournoud commentedI 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):
This makes no sense at all.
Comment #41
mfbThere's a notice Undefined index: time:normal when testing in sqlite. Presumably this is caused by typo 'time:nornaml'
Comment #42
seanburlington CreditAttribution: seanburlington commentedHappy 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?)
Comment #43
mfb@seanburlington see INSTALL.sqlite.txt (disclaimer I haven't looked at it in a while.. hope it works for you)
Comment #44
seanburlington CreditAttribution: seanburlington commentedThanks - 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.
Comment #46
mfbretesting
Comment #47
Crell CreditAttribution: Crell commentedI like simple.
Comment #48
webchickThis 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.
I don't know why the other three were spaced out with double-lines between them, but this one should match.
Comments should end in a period. Also, there's an extra blank line after the description that doesn't need to be there.
No t()s around stuff in getInfo().
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).
The assertion string should begin with a capital letter.
Please use proper placeholders. @time.
No need to double-quote this if there are no variables.
Comment #49
kjy07 CreditAttribution: kjy07 commentedRerolled per webchick.
Comment #50
kjy07 CreditAttribution: kjy07 commentedD'oh! Re(re)-rolled. :)
Comment #51
kjy07 CreditAttribution: kjy07 commentedComment #52
webchickThanks, 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
Comment #53
Crell CreditAttribution: Crell commentedIt has been documented.
Comment #55
jeffschulerTo help alleviate anyone else's confusion looking for this code, this change was rolled back in #866340: Remove support for date and time types.