Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17785 )
Change subject: IMPALA-2019(part-4): Add UTF-8 support for case conversion functions ...................................................................... Patch Set 6: (6 comments) Thanks for the feedback, Qifan! http://gerrit.cloudera.org:8080/#/c/17785/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17785/6//COMMIT_MSG@9 PS6, Line 9: string functions doing case conversion > nit. case conversion string functions Done http://gerrit.cloudera.org:8080/#/c/17785/6//COMMIT_MSG@11 PS6, Line 11: unicode > nit Unicode Done http://gerrit.cloudera.org:8080/#/c/17785/6/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/17785/6/be/src/exprs/string-functions-ir.cc@366 PS6, Line 366: if (wc_bytes == 0) break; > Per https://en.cppreference.com/w/cpp/string/multibyte/mbtowc, on return v Thanks for catching this! http://gerrit.cloudera.org:8080/#/c/17785/6/be/src/exprs/string-functions-ir.cc@370 PS6, Line 370: ; > nit. 0xFFFD. Done http://gerrit.cloudera.org:8080/#/c/17785/6/be/src/exprs/string-functions-ir.cc@371 PS6, Line 371: wc_bytes = 3; > nit. should it be 4? Thanks for catching this! I think we should jump to the next legal UTF-8 start byte. http://gerrit.cloudera.org:8080/#/c/17785/6/be/src/exprs/string-functions-ir.cc@408 PS6, Line 408: iswspace > nit. UNLIKELY() Done -- To view, visit http://gerrit.cloudera.org:8080/17785 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I443e89d46f4638ce85664b021666bc4f03ee8abd Gerrit-Change-Number: 17785 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 08 Feb 2022 08:06:42 +0000 Gerrit-HasComments: Yes
