Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13928 )
Change subject: KUDU-1938 Add non-copy setters to partial row pt 3 ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13928/1/src/kudu/common/partial_row.h File src/kudu/common/partial_row.h: http://gerrit.cloudera.org:8080/#/c/13928/1/src/kudu/common/partial_row.h@297 PS1, Line 297: used. This is : /// subject to change in the > I'm not sure what you mean, I haven't found anything that would mean the be The doxygen comments start with: /// @name Advanced/Unstable API // ///@{ And then end with: ///@} http://gerrit.cloudera.org:8080/#/c/13928/1/src/kudu/common/partial_row.h@307 PS1, Line 307: > I wanted to indicate that this is not for 'normal' as it doesn't guarantee When I read "Unsafe" I tend to think of memory accesses, or of APIs that plain aren't safe to use and should be avoided. Maybe NoTruncate would be more appropriate here? That's not perfect either though because it doesn't describe the lax length checking. Maybe just leave Unsafe then. http://gerrit.cloudera.org:8080/#/c/13928/1/src/kudu/common/partial_row.cc File src/kudu/common/partial_row.cc: http://gerrit.cloudera.org:8080/#/c/13928/1/src/kudu/common/partial_row.cc@423 PS1, Line 423: Status KuduPartialRow::SetCharNoCopyUnsafe(int col_idx, const Slice& val) { > Well, it would mean checking one character only which shouldn't be too expe Hmm, I guess I can see that. Could you doc this limitation in the CHAR doxygen comment? -- To view, visit http://gerrit.cloudera.org:8080/13928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f2aba098d649eb94e0314f6606cc33600e8d766 Gerrit-Change-Number: 13928 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 26 Jul 2019 20:40:55 +0000 Gerrit-HasComments: Yes
