[SQLite] Support timezone information on SqlitePlatform::getDateTimeTzFormatString()

[phansys]

Q	A
Type	improvement
Fixed issues	#5929

Summary

Do not discard timezone information on SQLite dates.

Closes #5929.

ToDo

• Add functional tests;
• Fix SQLite;
• Fix Oracle;
• Fix MySQL <- Support for timezone offsets was added in 8.0.19. Additionally, requires splitting of getDateTimeTzFormatString() (one for convertToDatabaseValue() and the other for convertToPHPValue()) because for INSERT / UPDATE the timezone offsets are allowed, but on SELECT the returned format is always an UTC timestamp (without offset), as it is how MySQL persists the timestamps;
• Fix MariaDB <- There is no support for timezones in timestamps (see this task and this task);
• Fix DB2 <- Ttimezone is not supported (see TIMESTAMP and String representations of datetime values
  );
• Check if docs should be updated for datetimetz and datetimetz_immutable types regarding platforms where the timezone offset information is lost during persistence (IE: PostgreSQL, MySQL >= 8.0.19).

[derrabus]

What would happen to existing apps with SQLite databases after upgrading to a version of DBAL that ships your change?

[derrabus]

Oh, and we need functional tests for this change. Can you investigate if there are already functional tests that cover datetime with TZ? Maybe those are currently skipped for SQLite?

[derrabus]

With all the issues that you're currently uncovering, I wonder if we should deprecate the current _tz types and replace them with something more consistent and tested across all platforms.

[phansys]

With all the issues that you're currently uncovering, I wonder if we should deprecate the current _tz types and replace them with something more consistent and tested across all platforms.

I agree that would be nice to have a more consistent behavior across platforms, but also, it depends on the behavior we expect about this type and how clear we are in our docs about the results in each case. I think we can indicate that the behavior is not "symmetric" but only works for writing (see #6014), as the same occurs at least with MySQL and PostgreSQL platforms.

Just to recap, these are the paths I think we have (just using a numbered list to make it referenceable):

1. Keep the tz types and throw exceptions (emit deprecations before 4.0) from getDateTimeTzTypeDeclarationSQL() and getDateTimeTzFormatString() (or any other related "tz" feature) in the platform if it doesn't support the timezone offsets, at least in an unidirectional way.
2. Update the docs and the functional tests to transmit and ensure a very clear understanding about each platform;
3. Make a conversion to the database's default timezone if the timezone offsets are not allowed (if this the same as the native "unidirectional" behavior provided by MySQL and PostgreSQL), optionally emitting a notice, warning or similar error;
4. Fall back silently to "non tz" types (datetime and datetime_immutable). This is the current behavior for all platforms.

   dbal/src/Platforms/AbstractPlatform.php

   Lines 4272 to 4292 in abb8f70

   	/**
   	* Gets the format string, as accepted by the date() function, that describes
   	* the format of a stored datetime value of this platform.
   	*
   	* @return string The format string.
   	*/
   	public function getDateTimeFormatString()
   	{
   	return 'Y-m-d H:i:s';
   	}
   	/**
   	* Gets the format string, as accepted by the date() function, that describes
   	* the format of a stored datetime with timezone value of this platform.
   	*
   	* @return string The format string.
   	*/
   	public function getDateTimeTzFormatString()
   	{
   	return 'Y-m-d H:i:s';
   	}

5. Remove (deprecate until 4.0) the "tz" types and provide the required timezone treatment in the "non tz" types based on the platform and the given input, throwing exceptions in cases where the provided values can not be properly handled.

Personally, I prefer 2 and 5,because that is what I expect from a DBAL: handle the value and process it or fail transparently if it is not possible.
Regarding the timestamps, my main concern is about the persisted value representing the right date and time, regardless the offset from where they were persisted or the offset from where they are obtained. In most cases (if not all), I think the timezone and offsets are used by the presentation layer, as it is usually desirable that UIs show the datetime representations according to the user's timezone. So, I don't think our timestamps need to persist those offsets.

[stof]

postgresql has 2 different types for timestamp with time zone and timestamp without time zone (the second one assumes that all timestamps are in the same time zone than the default time zone of the database server when doing date manipulation).

The existing timestamptz type works fine in postgresql and does the right thing about ensuring that the datetime correspond to the right point in time.
the DateTime object being returned for reads is not guaranteed to be using the same timezone than when writing, but anyway, the user to which we present the data might have a different time zone than the user who submitted the data. Avoiding to loose the timezone is actually the only way to handle this properly by allowing your presentation layer to perform the right timezone conversion when rendering the date (this part is indeed the responsibility of the view layer, but it works only if the data model does not loose its timezone so that the conversion is not garbage).

Removing the existing tz types would be a bad idea as it would break existing working situation for projects using a platform that supports those tz types without silently falling back to the non-tz types (i.e. Postgresql, Oracle or SQL Server).

Disclaimer: when saying that tz types work in those platforms, I'm not including whether getNowExpression supports the timestamptz type properly. This is indeed missing in a bunch of places but that method is deprecated anyway.

[stof]

Overriding getDateTimeTzFormatString without overriding getDateTimeTzTypeDeclarationSQL (so that it does not fallback to the non-tz type) will not provide a working implementation of the type (it will actually make things worse than doing the silent fallback consistently)

[phansys]

I'm afraid I'm missing the point here.
AFAIK, there are no specific types on MySQL for TIMESTAMP nor DATETIME:

MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval. (This does not occur for other types such as DATETIME).

[stof]

The point of the tz type is that it stores a value for a precise datetime, ensuring that when reading it again, we get the same point in time without having to agree on which timezone is implicitly used by the code.

For a tz type, we need to make sure that on insertion, we can provide a timezone and it is not ignored, but also that on reads, we can get a timezone info from the DB.
If MySQL works only with its implicit timezone on reads, it means we cannot support a tz type properly in DBAL (note that I'm not a fan of the fact that DBAL silently falls back to the non-tz type as it hides the fact that your db is not timezone aware anymore).

There is a good reason is the EcmaScript proposal for the Temporal API has separate objects representing ZonedDateTime and PlainDateTime.

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

[github-actions]

This pull request was closed due to inactivity.