PostgreSQL: Fix views\Tests\Handler\ArgumentDateTest

The Sql class in views handlers already has database driver code for handling date formatting, and it is correct.

For this query:
SELECT views_test_data.id AS id, views_test_data.id AS views_test_data_id FROM simpletest787317views_test_data views_test_data WHERE TO_CHAR((TO_TIMESTAMP(views_test_data.created) + INTERVAL '-14400 SECONDS'), 'YYYYMMDD') = '20000101';

Running this query manually after crashing a test and still having the test data:

SELECT *, views_test_data.id AS id, views_test_data.id AS views_test_data_id, TO_CHAR((TO_TIMESTAMP(views_test_data.created) + INTERVAL '-14400 SECONDS'), 'YYYYMMDD') as text FROM simpletest787317views_test_data views_test_data;

 id |   name   | age |    job     |  created  | status | id | views_test_data_id |   text   
----+----------+-----+------------+-----------+--------+----+--------------------+----------
  1 | John     |  25 | Singer     | 946684800 |      1 |  1 |                  1 | 19991231
  2 | George   |  27 | Singer     | 946771200 |      0 |  2 |                  2 | 20000101
  3 | Ringo    |  28 | Drummer    | 946708230 |      1 |  3 |                  3 | 20000101
  4 | Paul     |  26 | Songwriter | 946706400 |      0 |  4 |                  4 | 20000101
  5 | Meredith |  30 | Speaker    | 946708210 |      1 |  5 |                  5 | 20000101

The issue seems to be with the offset being set for timezone handling.

After an afternoon of debugging just in PostgreSQL I think the code is behaving as expected for offsets. I ran into 13 fails on MySQL when I had installed with time zone set to America/New_York. After I changed time zone of user and system to UTC, the 12/13 tests pass for PostgreSQL with the exception of Week.

So in order to decerease fails we can do what current testbot MySQL does - set time zone to UTC before install. :-)

I am going to open a new issue for this related to Views module.

The remaining fail is related to Week

Blargh, it's already on UTC. Silly wild goose chase in a different time zone :(

Same fail in SQLite.

I think this is because MySQL treats Jan 1 as week 52 so the test passes in 52 (as an integer even).

This should be "01" for PostgreSQL and "00" for SQLite.

Oracle seems to be an integer between 1 and 52 - http://docs.oracle.com/cd/B28359_01/olap.111/b28126/dml_commands_1029.htm
Datepart for SQL Server seems to also be an integer between 1 and 52.

This patch formats the week in the tests appropriately depending on the database type. I made the default an integer to try to help out non-core drivers.

I tested on SQLite and PostgreSQL locally.

Good work mradcliffe!

Your patch look good. But I have some remarks:

1. I think that it is good to add more tests to make sure that it also works for other years then only the year 2000.
2. Does these changes to the tests not need to be applied to the regular view argument date handler code.

Yes, I've been tossing that around in my head. The current behavior is for it to be the caller's responsibility to understand that the argument is relative to the database type and not ISO definition. This isn't documented anywhere. Changing that behavior means a bigger API change because we're suddenly changing what the caller is expecting. In my opinion, everyone should be expecting to pass in the ISO Week of Year instead of a relative one.

Another issue is that the current behavior expects to return multiple date ranges. If I pass in week 1, this will return all items where Unix timestamp is translated into the week of that particular year. So just passing in week of year 1 is not enough to get the MySQL or SQLite values because it could be different based on the Unix timestamp. The return values are always going to be different for database type and I do not think it is possible to come up with SQL to match all the permutations from the beginning to the end of time since each year is going to be different. I think that it would be a better expectation that the caller would want to pass in Week of current Year instead of Week of any Year. But again, I haven't found any documentation yet on why the current behavior is what it is.

For date/time formatting with PostgreSQL is the "WW" pattern used. This pattern does the following "week number of year (1-53) (The first week starts on the first day of the year.)".
In Drupal are we following the ISO week definition. The pattern for PostgreSQL is "IW". See the documentation.

I could have sworn that week 52 wasn't returning results, but I must have been doing something wrong in my debugging. :( I removed my incorrect summary from the summary.

I ran through the test with some debugging to look at the queries, and it worked properly.

Executed view: SELECT views_test_data.id AS id, views_test_data.id AS views_test_data_id
FROM 
{views_test_data} views_test_data
WHERE (( (TO_CHAR((TO_TIMESTAMP(views_test_data.created) + INTERVAL '0 SECONDS'), 'IW') = :views_test_data_date_week) ))
ORDER BY views_test_data_id ASC NULLS FIRST

Arguments: Array
(
    [:views_test_data_date_week] => 52
)

returned

Query:
SELECT views_test_data.id AS id, views_test_data.id AS views_test_data_id
FROM 
{views_test_data} views_test_data
WHERE (( (TO_CHAR((TO_TIMESTAMP(views_test_data.created) + INTERVAL '0 SECONDS'), 'IW') = :views_test_data_date_week) ))
ORDER BY views_test_data_id ASC NULLS FIRST

Query arguments:
array (
)

Actual result:
array (
  0 => 
  array (
    'id' => '1',
  ),
  1 => 
  array (
    'id' => '2',
  ),
  2 => 
  array (
    'id' => '3',
  ),
)

Expected result:
array (
  0 => 
  array (
    'id' => '1',
  ),
  1 => 
  array (
    'id' => '2',
  ),
  2 => 
  array (
    'id' => '3',
  ),
)

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed bd2e540 and pushed to 8.0.x. Thanks!

I guess sqlite still needs fixing but that is out-of-scope for this issue.