You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/02/04 00:27:49 UTC

[2/4] incubator-kudu git commit: KUDU-980 - Fix timestamp printing in c++

KUDU-980 - Fix timestamp printing in c++

The current time printing function, from gutil, had two problems:
- It only printed down to seconds.
- If it was passed 0 seconds, as it might in tests at least, it would
  ignore the argument and return the current time.

This makes our time printing like Impala's (though with less precision).
Specifically this makes us print time in the format:
"YYYY-MM-DD HH:MM:SS.ssssss GMT"

Change-Id: I97eb088a4d0ef082ab9c7e3cca40e809b0f29934
Reviewed-on: http://gerrit.cloudera.org:8080/965
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/72d4b044
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/72d4b044
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/72d4b044

Branch: refs/heads/master
Commit: 72d4b04477583238c2e2a60dc7cc27ad619722b8
Parents: cf1acc6
Author: David Alves <da...@cloudera.com>
Authored: Tue Sep 22 00:45:26 2015 -0700
Committer: David Ribeiro Alves <da...@cloudera.com>
Committed: Wed Feb 3 19:35:48 2016 +0000

----------------------------------------------------------------------
 src/kudu/common/CMakeLists.txt |  1 +
 src/kudu/common/types-test.cc  | 64 +++++++++++++++++++++++++++++++++++++
 src/kudu/common/types.h        | 29 +++++++++++++----
 3 files changed, 87 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/72d4b044/src/kudu/common/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/common/CMakeLists.txt b/src/kudu/common/CMakeLists.txt
index 48520e8..7cf9495 100644
--- a/src/kudu/common/CMakeLists.txt
+++ b/src/kudu/common/CMakeLists.txt
@@ -88,4 +88,5 @@ ADD_KUDU_TEST(row_changelist-test)
 ADD_KUDU_TEST(row_key-util-test)
 ADD_KUDU_TEST(row_operations-test)
 ADD_KUDU_TEST(schema-test)
+ADD_KUDU_TEST(types-test)
 ADD_KUDU_TEST(wire_protocol-test)

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/72d4b044/src/kudu/common/types-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/types-test.cc b/src/kudu/common/types-test.cc
new file mode 100644
index 0000000..ed56606
--- /dev/null
+++ b/src/kudu/common/types-test.cc
@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "kudu/common/types.h"
+
+using std::string;
+
+namespace kudu {
+
+TEST(TestTypes, TestTimestampPrinting) {
+  const TypeInfo* info = GetTypeInfo(TIMESTAMP);
+
+  // Test the minimum value
+  int64 time;
+  info->CopyMinValue(&time);
+  string result;
+  info->AppendDebugStringForValue(&time, &result);
+  ASSERT_EQ("-290308-12-21 19:59:05.224192 GMT", result);
+  result = "";
+
+  // Test a regular negative timestamp.
+  time = -1454368523123456;
+  info->AppendDebugStringForValue(&time, &result);
+  ASSERT_EQ("1923-12-01 00:44:36.876544 GMT", result);
+  result = "";
+
+  // Test that passing 0 microseconds returns the correct time (0 msecs after the epoch).
+  // This is a test for a bug where printing a timestamp with the value 0 would return the
+  // current time instead.
+  time = 0;
+  info->AppendDebugStringForValue(&time, &result);
+  ASSERT_EQ("1970-01-01 00:00:00.000000 GMT", result);
+  result = "";
+
+  // Test a regular positive timestamp.
+  time = 1454368523123456;
+  info->AppendDebugStringForValue(&time, &result);
+  ASSERT_EQ("2016-02-01 23:15:23.123456 GMT", result);
+  result = "";
+
+  // Test the maximum value.
+  time = MathLimits<int64>::kMax;
+  info->AppendDebugStringForValue(&time, &result);
+  ASSERT_EQ("294247-01-10 04:00:54.775807 GMT", result);
+}
+
+
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/72d4b044/src/kudu/common/types.h
----------------------------------------------------------------------
diff --git a/src/kudu/common/types.h b/src/kudu/common/types.h
index cf65f6c..470a12b 100644
--- a/src/kudu/common/types.h
+++ b/src/kudu/common/types.h
@@ -22,7 +22,7 @@
 
 #include <stdint.h>
 #include <string>
-#include <string.h>
+
 #include "kudu/common/common.pb.h"
 #include "kudu/gutil/mathlimits.h"
 #include "kudu/gutil/strings/escaping.h"
@@ -345,19 +345,34 @@ struct DataTypeTraits<STRING> : public DerivedTypeTraits<BINARY>{
   }
 };
 
+static const char* kDateFormat = "%Y-%m-%d %H:%M:%S";
+static const char* kDateMicrosAndTzFormat = "%s.%06d GMT";
+
 template<>
 struct DataTypeTraits<TIMESTAMP> : public DerivedTypeTraits<INT64>{
+  static const int US_TO_S = 1000L * 1000L;
+
   static const char* name() {
     return "timestamp";
   }
 
   static void AppendDebugStringForValue(const void* val, string* str) {
-    // TODO KUDU-980 - This only stringifies down to seconds,
-    // we should also print the micros.
-    time_t time = *reinterpret_cast<const int64_t *>(val);
-    char time_as_string[kFastToBufferSize];
-    FastTimeToBuffer(time, &time_as_string[0]);
-    str->append(time_as_string);
+    int64_t timestamp_micros = *reinterpret_cast<const int64_t *>(val);
+    time_t secs_since_epoch = timestamp_micros / US_TO_S;
+    // If the time is negative we need to take into account that any microseconds
+    // will actually decrease the time in seconds by one.
+    int remaining_micros = timestamp_micros % US_TO_S;
+    if (remaining_micros < 0) {
+      secs_since_epoch--;
+      remaining_micros = US_TO_S - std::abs(remaining_micros);
+    }
+    struct tm tm_info;
+    gmtime_r(&secs_since_epoch, &tm_info);
+    char time_up_to_secs[24];
+    strftime(time_up_to_secs, sizeof(time_up_to_secs), kDateFormat, &tm_info);
+    char time[34];
+    snprintf(time, sizeof(time), kDateMicrosAndTzFormat, time_up_to_secs, remaining_micros);
+    str->append(time);
   }
 };