[Asterisk-code-review] cel odbc: Fix timestamp processing for microseconds (asterisk[13])
Jenkins2
asteriskteam at digium.com
Wed May 10 06:32:56 CDT 2017
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5589 )
Change subject: cel_odbc: Fix timestamp processing for microseconds
......................................................................
cel_odbc: Fix timestamp processing for microseconds
When a column is of type timestamp, the fraction part of the event
field's seconds was frequently parsed incorrectly especially if
there were leading zeros. For instance "2017-05-23 23:55:03.023"
would be parsed into an int as "23" then when the timestamp was
formatted again to be inserted into the database column it'd be
"2017-05-23 23:55:03.23" which is now 230 milliseconds instead of
23 milliseconds. "03.000001" would be transformed to "03.1", etc.
* If the event field is 'eventtime' and the db column is timestamp,
then existing processing has already correctly formatted the
timestamp so now we simply use it rather than parsing it and
re-printing it. This is the most common use case anyway.
* If the event field is other than 'eventtime' and the db column
is timestamp, we now parse the seconds, including the fractional
part into a double rather than 2 ints. This preserves the
magnitude and precision of the fractional part. When we print
it, we now print it as a "%09.6lf" which correctly represents the
input.
To be honest, why we parse the string timestamp into components,
test the components, then print the components back into a string
timestamp is beyond me. We should use parse it, test it, then if
it passes, use the original string representation in the database
call. Maybe someone thought that some implementations wouldn't
take a partial timestamp string like "2017-05-06" and decided to
always produce a full timestamp string even if an abbreviated one
was supplied. Anyway, I'm leaving it as it is.
ASTERISK-25032 #close
Reported-by: Etienne Lessard
Change-Id: Id407e6221f79a5c1120e1a70bc7e893bbcaf1938
---
M cel/cel_odbc.c
1 file changed, 52 insertions(+), 29 deletions(-)
Approvals:
Mark Michelson: Looks good to me, but someone else must approve
Jenkins2: Approved for Submit
Joshua Colp: Looks good to me, approved
diff --git a/cel/cel_odbc.c b/cel/cel_odbc.c
index 1881231..1d304dc 100644
--- a/cel/cel_odbc.c
+++ b/cel/cel_odbc.c
@@ -37,6 +37,7 @@
#include <sys/types.h>
#include <time.h>
+#include <math.h>
#include <sql.h>
#include <sqlext.h>
@@ -616,40 +617,62 @@
if (ast_strlen_zero(colptr)) {
continue;
} else {
- int year = 0, month = 0, day = 0, hour = 0, minute = 0, second = 0, fraction = 0;
- if (strcasecmp(entry->name, "eventdate") == 0) {
- struct ast_tm tm;
- ast_localtime(&record.event_time, &tm, tableptr->usegmtime ? "UTC" : NULL);
- year = tm.tm_year + 1900;
- month = tm.tm_mon + 1;
- day = tm.tm_mday;
- hour = tm.tm_hour;
- minute = tm.tm_min;
- second = (tableptr->allowleapsec || tm.tm_sec < 60) ? tm.tm_sec : 59;
- fraction = tm.tm_usec;
+ if (datefield) {
+ /*
+ * We've already properly formatted the timestamp so there's no need
+ * to parse it and re-format it.
+ */
+ ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+ LENGTHEN_BUF2(27);
+ ast_str_append(&sql2, 0, "%s{ts '%s'}", first ? "" : ",", colptr);
} else {
- int count = sscanf(colptr, "%4d-%2d-%2d %2d:%2d:%2d.%6d", &year, &month, &day, &hour, &minute, &second, &fraction);
+ int year = 0, month = 0, day = 0, hour = 0, minute = 0;
+ /* MUST use double for microsecond precision */
+ double second = 0.0;
+ if (strcasecmp(entry->name, "eventdate") == 0) {
+ /*
+ * There doesn't seem to be any reference to 'eventdate' anywhere
+ * other than in this module. It should be considered for removal
+ * at a later date.
+ */
+ struct ast_tm tm;
+ ast_localtime(&record.event_time, &tm, tableptr->usegmtime ? "UTC" : NULL);
+ year = tm.tm_year + 1900;
+ month = tm.tm_mon + 1;
+ day = tm.tm_mday;
+ hour = tm.tm_hour;
+ minute = tm.tm_min;
+ second = (tableptr->allowleapsec || tm.tm_sec < 60) ? tm.tm_sec : 59;
+ second += (tm.tm_usec / 1000000.0);
+ } else {
+ /*
+ * If we're here, the data to be inserted MAY be a timestamp
+ * but the column is. We parse as much as we can.
+ */
+ int count = sscanf(colptr, "%4d-%2d-%2d %2d:%2d:%lf", &year, &month, &day, &hour, &minute, &second);
- if ((count != 3 && count != 5 && count != 6 && count != 7) || year <= 0 ||
- month <= 0 || month > 12 || day < 0 || day > 31 ||
- ((month == 4 || month == 6 || month == 9 || month == 11) && day == 31) ||
- (month == 2 && year % 400 == 0 && day > 29) ||
- (month == 2 && year % 100 == 0 && day > 28) ||
- (month == 2 && year % 4 == 0 && day > 29) ||
- (month == 2 && year % 4 != 0 && day > 28) ||
- hour > 23 || minute > 59 || second > (tableptr->allowleapsec ? 60 : 59) || hour < 0 || minute < 0 || second < 0 || fraction < 0) {
- ast_log(LOG_WARNING, "CEL variable %s is not a valid timestamp ('%s').\n", entry->name, colptr);
- continue;
+ if ((count != 3 && count != 5 && count != 6) || year <= 0 ||
+ month <= 0 || month > 12 || day < 0 || day > 31 ||
+ ((month == 4 || month == 6 || month == 9 || month == 11) && day == 31) ||
+ (month == 2 && year % 400 == 0 && day > 29) ||
+ (month == 2 && year % 100 == 0 && day > 28) ||
+ (month == 2 && year % 4 == 0 && day > 29) ||
+ (month == 2 && year % 4 != 0 && day > 28) ||
+ hour > 23 || minute > 59 || ((int)floor(second)) > (tableptr->allowleapsec ? 60 : 59) ||
+ hour < 0 || minute < 0 || ((int)floor(second)) < 0) {
+ ast_log(LOG_WARNING, "CEL variable %s is not a valid timestamp ('%s').\n", entry->name, colptr);
+ continue;
+ }
+
+ if (year > 0 && year < 100) {
+ year += 2000;
+ }
}
- if (year > 0 && year < 100) {
- year += 2000;
- }
+ ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+ LENGTHEN_BUF2(27);
+ ast_str_append(&sql2, 0, "%s{ts '%04d-%02d-%02d %02d:%02d:%09.6lf'}", first ? "" : ",", year, month, day, hour, minute, second);
}
-
- ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
- LENGTHEN_BUF2(27);
- ast_str_append(&sql2, 0, "%s{ts '%04d-%02d-%02d %02d:%02d:%02d.%d'}", first ? "" : ",", year, month, day, hour, minute, second, fraction);
}
break;
case SQL_INTEGER:
--
To view, visit https://gerrit.asterisk.org/5589
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id407e6221f79a5c1120e1a70bc7e893bbcaf1938
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list