Implementation of the datetime (with up to nanoseconds) has been added #1093
Conversation
There was a problem hiding this comment.
Sorry, but I couldn't finish reviewing the changes, partially because there are just too many of them clearly unrelated to the topic of this PR and I got discouraged when I saw that I covered barely half. It would be really great if you could please leave just the datetime-related parts.
I also still think that we should handle datetime as strings for the backends that don't support them natively, as otherwise this new type doesn't seem very useful.
We also need at least some documentation for the new type, especially if it's only supported in Oracle and PostgreSQL backends (which is not obvious).
| @@ -15,6 +15,8 @@ | |||
| #include <map> | |||
| #include <string> | |||
| #include <sstream> | |||
| #include <chrono> | |||
There was a problem hiding this comment.
These headers are not used, should be removed.
There was a problem hiding this comment.
Change to this file doesn't seem related to the PR topic.
There was a problem hiding this comment.
Change to this file doesn't seem related to the PR topic.
There was a problem hiding this comment.
Change to this file doesn't seem related to the PR topic.
| #ifndef NOMINMAX | ||
| # define NOMINMAX | ||
| #endif | ||
|
|
There was a problem hiding this comment.
This change doesn't seem related to the PR topic.
|
|
||
| default: | ||
| throw soci_error("Use element used with non-supported type."); |
There was a problem hiding this comment.
This change doesn't seem related to the PR topic.
There was a problem hiding this comment.
We do want to list all the enum elements explicitly instead of using default in order to get -Wshadow, so x_datetime should be added to the case statements here instead of removing them.
There was a problem hiding this comment.
Changes to this file don't seem related to the PR topic.
| os << "<vector>,"; | ||
| if ( ind_ ) | ||
| os << ind_->size (); | ||
| else | ||
| os << "null"; |
There was a problem hiding this comment.
This change doesn't seem related to the PR topic.
No description provided.