Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14705 )

Change subject: Added support of Kudu DATE type to Impala
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/14705/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14705/3//COMMIT_MSG@7
PS3, Line 7: Added
Can you create an Impala Jira and add it to the commit message?


http://gerrit.cloudera.org:8080/#/c/14705/3/be/src/exec/kudu-util.cc
File be/src/exec/kudu-util.cc:

http://gerrit.cloudera.org:8080/#/c/14705/3/be/src/exec/kudu-util.cc@136
PS3, Line 136: dv->ToString()
This will always return empty string if dv is invalid (and ToDaysSinceEpoch() 
would be true otherwise). I would simply return "Invalid DateValue.".


http://gerrit.cloudera.org:8080/#/c/14705/3/be/src/exec/kudu-util.cc@193
PS3, Line 193:     case TYPE_DATE:
The changes in this file all seem write related - was there any change on the 
read side? TimestampValue has some special logic, e.g. 
https://github.com/apache/impala/blob/f2f348c0f93208a0f34c33b6a4dc82f4d9d4b290/be/src/exec/kudu-scanner.cc#L336

DateValue's in memory representation is the some as in Kudu, so it is possible 
that less logic is needed than for Timestamps, but it is weird that nothing 
changed.



--
To view, visit http://gerrit.cloudera.org:8080/14705
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91656749a58ac769b54c2a63bdd4f85c89520b32
Gerrit-Change-Number: 14705
Gerrit-PatchSet: 3
Gerrit-Owner: Volodymyr Verovkin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Volodymyr Verovkin <[email protected]>
Gerrit-Comment-Date: Mon, 16 Dec 2019 14:53:48 +0000
Gerrit-HasComments: Yes

Reply via email to