Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19096 )
Change subject: IMPALA-11537: Query option validation for numeric types ...................................................................... Patch Set 12: Code-Review+1 (2 comments) Great work, thanks. http://gerrit.cloudera.org:8080/#/c/19096/12/be/src/service/query-option-parser.h File be/src/service/query-option-parser.h: http://gerrit.cloudera.org:8080/#/c/19096/12/be/src/service/query-option-parser.h@103 PS12, Line 103: static int64_t ConvertToInt(int64_t value) { return value; } I would add a comment also here describing the purpose of these functions. http://gerrit.cloudera.org:8080/#/c/19096/11/be/src/util/parse-util.h File be/src/util/parse-util.h: http://gerrit.cloudera.org:8080/#/c/19096/11/be/src/util/parse-util.h@48 PS11, Line 48: } > Removed It's fine if we don't log the original string but we could also store it as a value, I don't think that should take too much memory. But I'm fine with leaving that out. -- To view, visit http://gerrit.cloudera.org:8080/19096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7757b52393c094d2c661918d73cbfad7214f855 Gerrit-Change-Number: 19096 Gerrit-PatchSet: 12 Gerrit-Owner: Peter Rozsa <pro...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gergely Fürnstáhl <gfurnst...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Noemi Pap-Takacs <npaptak...@cloudera.com> Gerrit-Reviewer: Peter Rozsa <pro...@cloudera.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 12:59:48 +0000 Gerrit-HasComments: Yes