-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add DateFormat
function
#18
Conversation
Just formatting nothing in some cases is against this package's idea as it is trying to harmonize a concept to work exactly with each database. There are two ways to solve the problem:
|
Hello @tpetry .. Just pushed some progress on this,
|
I am not confident either. At the moment I've built all expressions based on the concept that I know how the implementations behave for every database. But with the date formatting that is much more complicated. Either I would have to manually test all these placeholder to check whether they match the theory. Or further extend the testing logic to also check the result of the SQL query. Is the table migration closure in the tests really needed? I guess all databases should have the datetime type. Edit: The special snowflake SQLite again doesn't have this type. Maybe I should add the testing then to use the blueprint for all tests. I'll have to think about it. |
4c49419
to
0457216
Compare
344aa62
to
2896f1a
Compare
Hello @tpetry, Sorry for the time it took me to revisit my approach to this and share my updates. Most of the characters are now supported (Full support table below). But the PR size has increased. It's totally understandable if you're having second thoughts about adding this functionality.
A major change was moving away from pushing as many characters as possible to the date format function, to concat each character and only map one character at a time. This will result in a longer SQL statement, Reasons for this: the DBMS behavior differs towards the additional non-format characters, or if the format ended with spaces or dashes or something else. Unfortunately, I've lost the exact test cases where I've noticed this issue. Also, after cleaning up from that approach, the code got simplified (Got to remove these lines) so I opted to keep it.
Support table
script to generate the table#!/usr/bin/env php
<?php
require __DIR__.'/../vendor/autoload.php';
use Tpetry\QueryExpressions\Function\Date\DateFormat;
$dateFormats = [
'd' => 'Day of the month, 2 digits with leading zeros (01 to 31)',
'D' => 'A textual representation of a day, three letters (Mon through Sun)',
'j' => 'Day of the month without leading zeros (1 to 31)',
'l' => 'A full textual representation of the day of the week (Sunday through Saturday)',
'N' => 'ISO 8601 numeric representation of the day of the week (1 for Monday through 7 for Sunday)',
'S' => 'English ordinal suffix for the day of the month, 2 characters (st, nd, rd, or th)',
'w' => 'Numeric representation of the day of the week (0 for Sunday through 6 for Saturday)',
'z' => 'The day of the year (starting from 0) (0 through 365)',
'W' => 'ISO 8601 week number of the year, weeks starting on Monday (Example: 42)',
'F' => 'A full textual representation of a month (January through December)',
'm' => 'Numeric representation of a month, with leading zeros (01 through 12)',
'M' => 'A short textual representation of a month, three letters (Jan through Dec)',
'n' => 'Numeric representation of a month, without leading zeros (1 through 12)',
't' => 'Number of days in the given month (28 through 31)',
'L' => 'Whether it\'s a leap year (1 if it is a leap year, 0 otherwise)',
'o' => 'ISO 8601 week-numbering year (Examples: 1999 or 2003)',
'X' => 'An expanded full numeric representation of a year, at least 4 digits, with - for years BCE, and + for years CE (Examples: -0055, +0787, +1999, +10191)',
'x' => 'An expanded full numeric representation if required, or a standard full numeral representation if possible (like Y). At least four digits. Years BCE are prefixed with a -. Years beyond (and including) 10000 are prefixed by a + (Examples: -0055, 0787, 1999, +10191)',
'Y' => 'A full numeric representation of a year, at least 4 digits, with - for years BCE (Examples: -0055, 0787, 1999, 2003, 10191)',
'y' => 'A two-digit representation of a year (Examples: 99 or 03)',
'a' => 'Lowercase Ante meridiem and Post meridiem (am or pm)',
'A' => 'Uppercase Ante meridiem and Post meridiem (AM or PM)',
'B' => 'Swatch Internet time (000 through 999)',
'g' => '12-hour format of an hour without leading zeros (1 through 12)',
'G' => '24-hour format of an hour without leading zeros (0 through 23)',
'h' => '12-hour format of an hour with leading zeros (01 through 12)',
'H' => '24-hour format of an hour with leading zeros (00 through 23)',
'i' => 'Minutes with leading zeros (00 to 59)',
's' => 'Seconds with leading zeros (00 through 59)',
'u' => 'Microseconds (Note: date() will always generate 000000 since it takes an int parameter, whereas DateTime::format() does support microseconds if DateTime was created with microseconds. Example: 654321)',
'v' => 'Milliseconds (Same note applies as for u. Example: 654)',
'e' => 'Timezone identifier (Examples: UTC, GMT, Atlantic/Azores)',
'I' => 'Whether or not the date is in daylight saving time (1 if Daylight Saving Time, 0 otherwise)',
'O' => 'Difference to Greenwich time (GMT) without colon between hours and minutes (Example: +0200)',
'P' => 'Difference to Greenwich time (GMT) with colon between hours and minutes (Example: +02:00)',
'p' => 'The same as P, but returns Z instead of +00:00 (available as of PHP 8.0.0. Examples: Z or +02:00)',
'T' => 'Timezone abbreviation, if known; otherwise, the GMT offset (Examples: EST, MDT, +05)',
'Z' => 'Timezone offset in seconds (The offset for timezones west of UTC is always negative, and for those east of UTC is always positive. -43200 through 50400)',
'c' => 'ISO 8601 date (2004-02-12T15:19:21+00:00)',
'r' => 'RFC 2822/RFC 5322 formatted date (Example: Thu, 21 Dec 2000 16:01:07 +0200)',
'U' => 'Seconds since the Unix Epoch (January 1, 1970, 00:00:00 GMT, See also time())',
];
$output = "| .. Format .. | MySQL | SQLite | PostgreSQL | SQL Server | Full Support |\n";
$output .= "| --- | --- | --- | --- | --- | --- |\n";
$class = new ReflectionClass(DateFormat::class);
$emulatableCharacters = $class->getDefaultProperties()['emulatableCharacters'];
$formatCharacters = $class->getDefaultProperties()['formatCharacters'];
foreach ($dateFormats as $character => $description) {
$output .= "| <details><summary>`{$character}`</summary>{$description}</details> | ";
$fullSupport = true;
foreach (['mysql', 'sqlite', 'pgsql', 'sqlsrv'] as $driver) {
$format = $formatCharacters[$driver][$character] ?? null;
$emulatableCharacter = $emulatableCharacters[$driver][$character] ?? null;
if ($format) {
$output .= "`$format`";
} elseif ($emulatableCharacter) {
$output .= "<details><summary>🔗️️</summary>`$emulatableCharacter`</details>";
} else {
$output .= '❌';
$fullSupport = false;
}
$output .= ' | ';
}
$output .= $fullSupport ? '✅' : '❌';
$output .= "\n";
}
echo $output; |
Thanks for the big PR. But I'll need quite some time to go through it completely. Shouldn't we throw exceptions when unsupported date formats are used? |
@MohannadNaj You're still working on the date format functionality in your fork. So is this code still a draft as you have new ideas to further improve it? |
@tpetry Yes, some changes are still in progress in another branch here. I also prefer to close this PR and re-submit in a new PR when it's ready. It's another branch because I'm not sure if you agree with the approach I'm taking in it (Mainly, reusing expressions in the emulated queries), will make it into this branch after your initial feedback. (Thanks in advance 🙏). Done:
Done but needs your approval/comment:
For example instead of Sqlite's - 'A' => '(CASE WHEN STRFTIME(\'%%H\', %s) < \'12\' THEN \'AM\' ELSE \'PM\' END)',
+ 'A' => (new CaseGroup(
+ when: [
+ new CaseRule(
+ new Value('AM'),
+ new LessThan(
+ new QueryExpression('STRFTIME(\'%%H\', %s)'),
+ new Value('12'),
+ )
+ ),
+ ],
+ else: new Value('PM'),
+ )), Do we want this? it's resulting in a larger code base (Current EmulatedDateFormat trait is +480 lines).. It's possible to just revert it to a long array with the direct SQL string, but reusing the package expressions feels cleaner/safer.
Still wip:
Yes I agree.
|
I am still unsure about this approach. I see a problem that all test cases are failing when dependent expressions are changed. I think I prefer not building expressions on-top of others because of this.
A
Yeah, localization is a big problem. I haven't yet tested all the different databases. Could we provide a language to use for the expression? Or could it just be the task for the user to set the appropriate language within the database connection? |
2896f1a
to
53842bf
Compare
53842bf
to
335b0b5
Compare
Closing for a re-submission in another PR.
I've used Carbon's default locale to produce the textual formats, this way we won't have to maintain these values, which turned out to be only used by our dear SQLite lol, other DBMSs have a built-in way of producing such formats. If the user uses SQLite and wants a different locale, that should happen by |
Following up on #15 and the discussion by @phillipfickl #7 (comment)
Used resources:
https://github.com/drupal/drupal/blob/eafbd246a944b20d1f507cae159fd9574440512e/core/modules/views/src/Plugin/views/query/MysqlDateSql.php#L27-L45
https://github.com/drupal/drupal/blob/eafbd246a944b20d1f507cae159fd9574440512e/core/modules/views/src/Plugin/views/query/PostgresqlDateSql.php#L32-L51