[Maria-developers] MDEV-11829 RFC3339 Support
Hello, This is in response to the comments on the initial pull request I made for adding RFC3339 support [1]. The proposal is to add support for parsing RFC3339 and then use the given timezone offset to shift the datetime. I think most users would expect the behavior to be converted to the timezone of the mariadb server / session as defined in THD. Most timezone operations are depended on the session specified timezone, whether that is set or inherited from global timezone, or system timezone. In order to convert from RFC3339 timezone to session timezone, we will need to return the timezone from the str_to_datetime function. As mentioned there is no timezone member of the MY_TIME struct. I believe a change to the MY_TIME struct would be the most beneficial and cleanest approach. This would allow for handling RFC3339 and future additions, such as a datetime with time zone field, similar to the ansi timestamp with time zone. The struct is used in a several ways, including packing into longlong, all of those cases would need to be handled. This would be a much larger and more involved patchset. An alternative approach, that is less involved, would be use the MYSQL_TIME_STATUS struct to indicate that a timezone offset exists. An offset field and indicator could be added right to the status struct, or a new warning can be used to indicate it exists in the raw string. Then the caller of str_to_datetime can handle the timezone conversion. A third approach, would be to change the function signature of str_to_datetime and provide a variable to store the offset in, if exists. I'm sure what the general stance is on altering entire function calls, especially one that is older and quite important. With any approach, the timezone could then be handled outside of my_time.c, where we have THD and the timezone class, which can easily handle the conversion. I will start working on this once I receive some feedback on the general direction that would result in an acceptable patchset. [1] https://github.com/MariaDB/server/pull/290 Thank you, Seth Shelnutt
Hi, Seth! On Feb 02, Seth Shelnutt wrote:
Hello,
This is in response to the comments on the initial pull request I made for adding RFC3339 support [1].
The proposal is to add support for parsing RFC3339 and then use the given timezone offset to shift the datetime. I think most users would expect the behavior to be converted to the timezone of the mariadb server / session as defined in THD. Most timezone operations are depended on the session specified timezone, whether that is set or inherited from global timezone, or system timezone.
It's not what SQL standard specifies. It says, <...> whenever a datetime value without time zone is to be implicitly derived from one with, SQL assumes the value with time zone to be UTC, adds the time zone displacement to it to give local time, and the result, without any time zone displacement, is local. This doesn't make much sense to be, really, because it totally ignores the session time zone. On the other hand, if you'd convert the UTC value to be in the session time zone, you'd totally ignore the displacement value. I don't see any behavior being better than the other, so I'd stick to the standard by default.
In order to convert from RFC3339 timezone to session timezone, we will need to return the timezone from the str_to_datetime function. As mentioned there is no timezone member of the MY_TIME struct. I believe a change to the MY_TIME struct would be the most beneficial and cleanest approach. This would allow for handling RFC3339 and future additions, such as a datetime with time zone field, similar to the ansi timestamp with time zone. The struct is used in a several ways, including packing into longlong, all of those cases would need to be handled. This would be a much larger and more involved patchset.
To support TIMESTAMP WITH TIME ZONE and TIME WITH TIME ZONE literals (literals only, not expressions or fields or variables), it would be enough to parse them in str_to_datetime(), there is no need to return the time zone back to the caller in MY_TIME.
With any approach, the timezone could then be handled outside of my_time.c, where we have THD and the timezone class, which can easily handle the conversion.
There is no need to handle time zone outside of my_time.c if all we want is TIMESTAMP WITH TIME ZONE literals with the standard conversion behavior. It can be implemented completely inside str_to_datetime. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Sergei, Thank you for your time and responses. On Tue, Feb 14, 2017 at 3:11 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Seth!
It's not what SQL standard specifies. It says,
<...> whenever a datetime value without time zone is to be implicitly derived from one with, SQL assumes the value with time zone to be UTC, adds the time zone displacement to it to give local time, and the result, without any time zone displacement, is local.
This doesn't make much sense to be, really, because it totally ignores the session time zone. On the other hand, if you'd convert the UTC value to be in the session time zone, you'd totally ignore the displacement value. I don't see any behavior being better than the other, so I'd stick to the standard by default.
In order to convert from RFC3339 timezone to session timezone, we will need to return the timezone from the str_to_datetime function. As mentioned there is no timezone member of the MY_TIME struct. I believe a change to the MY_TIME struct would be the most beneficial and cleanest approach. This would allow for handling RFC3339 and future additions, such as a datetime with time zone field, similar to the ansi timestamp with time zone. The struct is used in a several ways, including packing into longlong, all of those cases would need to be handled. This would be a much larger and more involved patchset.
To support TIMESTAMP WITH TIME ZONE and TIME WITH TIME ZONE literals (literals only, not expressions or fields or variables), it would be enough to parse them in str_to_datetime(), there is no need to return the time zone back to the caller in MY_TIME.
In adding a gmt_offset variable, and boolean flag to MY_TIME struct it will break abi compatibility. What are the standards around abi compatibility changes? Would adding all relevant changes behind #ifdef be sufficient? In that case, at compile time a user could choose to maintain existing abi compatibility, or choose to enable TIME(STAMP) with TIME ZONE literal support?
With any approach, the timezone could then be handled outside of my_time.c, where we have THD and the timezone class, which can easily handle the conversion.
There is no need to handle time zone outside of my_time.c if all we want is TIMESTAMP WITH TIME ZONE literals with the standard conversion behavior. It can be implemented completely inside str_to_datetime.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Thank you, Seth Shelnutt
Hi, Seth! On Feb 21, Seth Shelnutt wrote:
To support TIMESTAMP WITH TIME ZONE and TIME WITH TIME ZONE literals (literals only, not expressions or fields or variables), it would be enough to parse them in str_to_datetime(), there is no need to return the time zone back to the caller in MY_TIME.
In adding a gmt_offset variable, and boolean flag to MY_TIME struct it will break abi compatibility. What are the standards around abi compatibility changes?
We change the ABI, we need to change the library version. That's a lot of work, old clients will need the old library, we'll maintain *-compat package with old library versions, etc.
Would adding all relevant changes behind #ifdef be sufficient? In that case, at compile time a user could choose to maintain existing abi compatibility, or choose to enable TIME(STAMP) with TIME ZONE literal support?
No, #ifdef in the ABI is a very bad idea. A user will never know whether the application will work or it'll crash. The library name and a version should always identify the ABI unambiguously. What do you want to achieve at the end? What's your use case? If it's *only* reading timestamps with a time zone, that is only literals for insert/update/etc - then you can do with a rather small fix, no ABI changes, no new data types, no on-disk format changes, nothing. A full support, a new data type TIMESTAMP WITH TIME ZONE - is significantly more complex, though. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hello Seth, I added a comment into the pull request: https://github.com/MariaDB/server/pull/290 Please have a look. Thanks! On 02/21/2017 04:37 PM, Seth Shelnutt wrote:
Sergei,
Thank you for your time and responses.
On Tue, Feb 14, 2017 at 3:11 PM Sergei Golubchik <serg@mariadb.org <mailto:serg@mariadb.org>> wrote:
Hi, Seth!
It's not what SQL standard specifies. It says,
<...> whenever a datetime value without time zone is to be implicitly derived from one with, SQL assumes the value with time zone to be UTC, adds the time zone displacement to it to give local time, and the result, without any time zone displacement, is local.
This doesn't make much sense to be, really, because it totally ignores the session time zone. On the other hand, if you'd convert the UTC value to be in the session time zone, you'd totally ignore the displacement value. I don't see any behavior being better than the other, so I'd stick to the standard by default.
> In order to convert from RFC3339 timezone to session timezone, we will > need to return the timezone from the str_to_datetime function. As > mentioned there is no timezone member of the MY_TIME struct. I believe > a change to the MY_TIME struct would be the most beneficial and > cleanest approach. This would allow for handling RFC3339 and future > additions, such as a datetime with time zone field, similar to the > ansi timestamp with time zone. The struct is used in a several ways, > including packing into longlong, all of those cases would need to be > handled. This would be a much larger and more involved patchset.
To support TIMESTAMP WITH TIME ZONE and TIME WITH TIME ZONE literals (literals only, not expressions or fields or variables), it would be enough to parse them in str_to_datetime(), there is no need to return the time zone back to the caller in MY_TIME.
In adding a gmt_offset variable, and boolean flag to MY_TIME struct it will break abi compatibility. What are the standards around abi compatibility changes? Would adding all relevant changes behind #ifdef be sufficient? In that case, at compile time a user could choose to maintain existing abi compatibility, or choose to enable TIME(STAMP) with TIME ZONE literal support?
> With any approach, the timezone could then be handled outside of > my_time.c, where we have THD and the timezone class, which can easily > handle the conversion.
There is no need to handle time zone outside of my_time.c if all we want is TIMESTAMP WITH TIME ZONE literals with the standard conversion behavior. It can be implemented completely inside str_to_datetime.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org <mailto:security@mariadb.org>
Thank you,
Seth Shelnutt
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
participants (3)
-
Alexander Barkov -
Sergei Golubchik -
Seth Shelnutt