Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/13760 )
Change subject: KUDU-1938 Add support for CHAR/VARCHAR pt 1 ...................................................................... Patch Set 19: (3 comments) http://gerrit.cloudera.org:8080/#/c/13760/19/src/kudu/util/char_util.h File src/kudu/util/char_util.h: http://gerrit.cloudera.org:8080/#/c/13760/19/src/kudu/util/char_util.h@44 PS19, Line 44: <==> > What's this mean? tried to denote "if and only if" with this. Deleting this method anyway for now as it's easier to do this whole thing as part of UTF8Truncate and it's not used anywhere else now anyway without UTF8Resize. http://gerrit.cloudera.org:8080/#/c/13760/19/src/kudu/util/char_util.h@54 PS19, Line 54: std::string UTF8Resize(Slice val, size_t utf8_length); > For simplicity, could UTF8Truncate be folded into UTF8Resize? Meaning, coul yeah I think I originally I split them because I needed an std::string in one case and Slice in the other. I think we don't even need this as we're not planning to pad, only to truncate, so I'll go ahead and remove this method for now. http://gerrit.cloudera.org:8080/#/c/13760/19/src/kudu/util/char_util.cc File src/kudu/util/char_util.cc: http://gerrit.cloudera.org:8080/#/c/13760/19/src/kudu/util/char_util.cc@44 PS19, Line 44: Slice UTF8Truncate(Slice val, size_t max_utf8_length) { > Should DCHECK that max_utf8_length < val.size()? I don't think we need to, calling this method with max_utf8_length >= val.size() should be valid, in which case it's not truncated, only copied. At least that's how I use it right now as we need to memcpy val.data() either way. -- To view, visit http://gerrit.cloudera.org:8080/13760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I998982dba93831db91c43a97ce30d3e68c2a4a54 Gerrit-Change-Number: 13760 Gerrit-PatchSet: 19 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Wed, 17 Jul 2019 16:35:48 +0000 Gerrit-HasComments: Yes
