Shirish Gajera has posted comments on this change. Change subject: IMPALA-2033: Netezza compatibility functions quote_ident ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/3263/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 1958: TestStringValue("quote_ident('100')", "\"100\""); > Can you test something starting with a number and ending with alpha. Done http://gerrit.cloudera.org:8080/#/c/3263/1/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: PS1, Line 761: std:: > You should be able to use string without the std:: prefix. The "common/name Done PS1, Line 767: numeric > This is really non_alpha, right? Done Line 768: if (all_lowercase.find_first_not_of(lowercase_numeric) == std::string::npos) { > Maybe use && instead of nested if statements? Done Line 769: if (all_lowercase.find_first_not_of(numeric) != std::string::npos) { > Shouldn't this be checking for whether the first character is numeric? Done -- To view, visit http://gerrit.cloudera.org:8080/3263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idbac4cd10fe08a6bd4871a03ba3b5f2c20fa8ee1 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Shirish Gajera <[email protected]> Gerrit-Reviewer: Shirish Gajera <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
