Skip to content

TimeOnly constructors support #321

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

Merged
merged 26 commits into from
Apr 11, 2023
Merged

TimeOnly constructors support #321

merged 26 commits into from
Apr 11, 2023

Conversation

alex-kulakov
Copy link
Contributor

Adds support for TimeOnly constructors (from parts and from ticks) in queries for majority of providers.

Some providers (or versions of them) require special and not the best way to construct TIME values but save way in terms of value range. If there is a function to create a time value, we use it (e.g. MS SQL Server from v11, PostgreSQL from v10). In other cases, there are following ways:

  1. add all time parts one by one to 00:00:00.000 time value;
  2. compose a string of time value and then cast it to time.

Both of them have certain overhead but the second one provides boundaries check and an exception will be thrown if cast from string fails, which is good. The first way will have value overflow problem when there are more than 24 hours, which is possible.

Cases when TimeSpan.Ticks is used as parameter value for TimeOnly(ticks) constructor are handled additionally, to have less calculations.

Excluded providers are: SQLite, MySQL providers. These RDBMSs have certain particularities which were disqualifying.
SQLite: TIME values can have range, which is wider than TimeOnly's range, for example, 24:05.xx.xxx is valid value. The other part of problem is that there is no way to guarantee correct value even if it is wrong (25:05:xx.xxx). Storage will return NULL instead of exception of some sort. NULL value may be valid in case of Nullable<TimeOnly> persistent field which may cause wrong results of query. Value overflow is not an option because it also may lead to unexpected results.
MySQL: TIME value have range from -838:59:59.000000 to 838:59:59.000000, which is part of the problem the second part is that even if hour part is 839, the storage does not throw any errors but return max value. If there was an overflow error we could mathematically overcome this problem by adding 815 hours and then subtracting this value from result time value, but we can't. Newer versions have MAKE_TIME function which also doesn't control parameters, even if string is used as value, it will return NULL which may be valid value.

+ Interval type has correct natilve type association (was already in use, see v11.Translator)
There is no way to control hours overflow, Even MAKETIME function
returns NULL or max value if value is incorrect, this opens way to
possibly corrupted query results which is bad
There is no way to control hours overflow, STRFTIME
returns NULL  if value is incorrect, this opens way to
possibly corrupted query results which is bad.
+ ignored SQLite and MySQL in test for second variant of TimeConstruct
@alex-kulakov
Copy link
Contributor Author

@SergeiPavlov do your team have any problems with this?

@SergeiPavlov
Copy link
Contributor

@SergeiPavlov do your team have any problems with this?

AFAIK - no

@alex-kulakov alex-kulakov merged commit 566de7c into master Apr 11, 2023
@alex-kulakov alex-kulakov deleted the master-timeonly-ticks branch April 11, 2023 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants