Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-5338: Fix Kudu timestamp column default values ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/6936/1/be/src/runtime/timestamp-value.h File be/src/runtime/timestamp-value.h: Line 108: static TimestampValue FromUnixTimeMicros(int64_t unix_time_micros); I'm confused as to why it no longer takes two parameters (maybe its just that the comment needs to be updated?) http://gerrit.cloudera.org:8080/#/c/6936/1/be/src/runtime/timestamp-value.inline.h File be/src/runtime/timestamp-value.inline.h: Line 32: int64_t ts_seconds = unix_time_micros / MICROS_PER_SEC; I don't understand what this is supposed to be doing - it looks like you're just separating 'unix_time_micros' into two parts and then adding them back together again. http://gerrit.cloudera.org:8080/#/c/6936/1/fe/src/main/java/org/apache/impala/catalog/KuduColumn.java File fe/src/main/java/org/apache/impala/catalog/KuduColumn.java: PS1, Line 125: value extraneous word? http://gerrit.cloudera.org:8080/#/c/6936/1/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test File testdata/workloads/functional-query/queries/QueryTest/kudu_create.test: Line 253: ts2 timestamp default cast('00:00:00' as timestamp)) I guess the intention here is that this will fail? Is there an error we can catch? http://gerrit.cloudera.org:8080/#/c/6936/1/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: Line 364: schema_builder = SchemaBuilder() comment Line 658: e TIMESTAMP NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION DEFAULT timestamp_from_unix_micros(%s), long line -- To view, visit http://gerrit.cloudera.org:8080/6936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I655910fb4805bb204a999627fa9f68e43ea8aaf2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-HasComments: Yes
