Tim Armstrong has posted comments on this change. Change subject: IMPALA-2033: Netezza compatibility functions quote_ident ......................................................................
Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/3263/4/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: Line 761: string identifiers(reinterpret_cast<const char*>(str.ptr), str.len); It seems wasteful to copy this into a string. I don't think we even use this string aside from copying it again in the next line. Line 772: const string regular_identifiers("ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_$"); This should be a static const string defined outside the function. I'm pretty sure that using std::string like this will can/will malloc() memory every time the function is executed. I.e. static const string NETEZZA_REGULAR_IDENTIFIER_CHARS... Line 774: case_insensitive_identifiers.find_first_not_of(regular_identifiers) == string::npos) { Long line (should be 90 chars or less) -- 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: 4 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
