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

Reply via email to