Improved cross-database SQL date handling [#239380]

Attached is a patch to implement cross-database SQL date handling. It implements a functions for converting timestamps to date fields, creating the appropriate format strings for SQL date formating, and the appropriate code for SQL date extraction.

It's taken in part from the Date API, so has received some testing, but it will still need more work. It will at least get all the date manipulation in a single place where it can more easily be fixed when we find problems. and it provides a starting point for code that will work in POSTGRESQL, too.

If you're interested, we could probably back-port this to Views 1, too.

Comment	File	Size	Author
#5	views.patch	9.66 KB	KarenS
#5
#4	views.patch	9.66 KB	KarenS
#4
	views.patch	7.99 KB	KarenS

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

KarenS CreditAttribution: KarenS commented 27 March 2008 at 14:49

Oh, and it also introduces a way to manipulate dates that are already in native datetime format, as well as timestamps, since we now have that field type available in the Schema API.

Comment #2

merlinofchaos CreditAttribution: merlinofchaos commented 27 March 2008 at 15:00

As usual, Karen, this is freakin' awesome.

One comment: The places with ***table*** need to preserve that. So It should probably be sending "***table***.$this->real_field" through the function call.

The reason for this is important: No handler actually knows the name of the table it's using, because application of a relationship can cause the table to be aliased. But at the time the formula is defined, it's too early to know what that alias is. So the formula handler is using a replacement in query() to let us stick ***table*** in as a token.

Otherwise, this looks really really good.

Comment #3

mooffie CreditAttribution: mooffie commented 27 March 2008 at 16:11

Just for the record (and excuse me if it's not the place here, but it seems relevant): I've been working on a library that provides a thin SQL layer for dealing with dates. It looks like:

$expr = date_sql_factory();

$expr->start_with('n.created', 'timestamp')
     ->add(1, 'month')
     ->truncate_to('days')
     ->coerce_to('string:iso16');

$sql = "SELECT ". $expr->get() ." FROM {node} n";

//////////////
// A method to group by year:

$expr->start_with('some_table.some_field', 'string:iso16T')
     ->extract('year');

$sql = "SELECT ". $expr->get() ." AS year FROM {some_table} GROUP BY year";

The chaining concept is borrowed from jQuery (Unfortunately, this chaining doesn't work in PHP4, but you can do without).

However, I got stuck with timezone handling. The problem is that Postgres and MySQL have a radically different approach for handling timezones and I have yet to find a model that bridges the differences.

Comment #4

KarenS CreditAttribution: KarenS commented 27 March 2008 at 16:27

File	Size
views.patch	9.66 KB

Here's my updated patch. I added in a way to control the offset when necessary, which I need in the Date module for dates that have date-specific timezone offsets and which the Event module may need because it stores the local date instead of the UTC date. With this patch, I can get rid of this code in the Date API, because the only place it's used is in Views.

@moofie, I did a lot of work trying to get timezone handling done in the database in the Date module and found that not only is there no good cross-database way to do it, but we can't count on the necessary timezone tables being installed in MYSQL. Many many people reported that their hosts did NOT have the timezone tables installed, even in current versions of MYSQL. So I have concluded that we're stuck with doing this by adding offsets to the UTC value.

Comment #5

KarenS CreditAttribution: KarenS commented 28 March 2008 at 13:52

File	Size
views.patch	9.66 KB

Had a missing semi-colon in the last patch, so I re-rolled it.

moofie, are you proposing to rewrite this patch using your factory() method? It seems like a nice system, but it will need to do what this patch does -- incorporate adjustments for offsets, allow for offsets that are coming from the database as field names instead of values, etc. I know Earl is planning to get a beta version of Views released very soon, so I was trying to move quickly to get this in, but if you plan to provide an alternate patch, just say so.

Comment #6

merlinofchaos CreditAttribution: merlinofchaos commented 28 March 2008 at 17:38

Status:

Needs review

» Fixed

Committed, though the last patch still lacked a semi-colon =)

Comment #7

mooffie CreditAttribution: mooffie commented 30 March 2008 at 04:33

moofie, are you proposing to rewrite this patch using your factory() method?

No, I wasn't proposing anything. Sorry if I unintentionally mislead anybody (and sorry for not replying earlier): I was just showing what I had on my mind for the (far) future. Of course your patch is fine.

Karen, you're doing an amazing work on Drupal. Thanks.

@moofie, I did a lot of work trying to get timezone handling done in the database in the Date module and found that not only is there no good cross-database way to do it, but we can't count on the necessary timezone tables being installed in MYSQL

(I didn't consider this the problem; after all, you can easily detect the existence of these table. In a nutshell, the problem as I see it is that MySQL doesn't really support timezones: you have to do convert_tz() explicitly, whereas in Postgres the timezone is carried with the value and conversion is done implicitly. But let's leave this #239380 issue to rest ;-)