Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20447 )
Change subject: IMPALA-13039: AES Encryption/ Decryption Support in Impala ...................................................................... Patch Set 28: (4 comments) http://gerrit.cloudera.org:8080/#/c/20447/28/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/20447/28/be/src/exprs/string-functions-ir.cc@1907 PS28, Line 1907: try { We generally try to avoid exceptions, especially in hot code paths like per-row functions and for regular control flow. This would work just fine by returning a Status object and passing encryption_key as an argument to fill. http://gerrit.cloudera.org:8080/#/c/20447/28/be/src/udf/udf.h File be/src/udf/udf.h: http://gerrit.cloudera.org:8080/#/c/20447/28/be/src/udf/udf.h@238 PS28, Line 238: void SetLoggerFlag(); Have you verified each UDF gets a unique FunctionContext? nit: This API isn't very versatile, it only allows on/off rather than tracking distinct log messages. Could you use Set/GetFunctionState instead? http://gerrit.cloudera.org:8080/#/c/20447/28/be/src/udf/udf.h@268 PS28, Line 268: bool logger_flag = false; This seems like it should live in FunctionContextImpl. http://gerrit.cloudera.org:8080/#/c/20447/25/be/src/util/openssl-util-test.cc File be/src/util/openssl-util-test.cc: http://gerrit.cloudera.org:8080/#/c/20447/25/be/src/util/openssl-util-test.cc@91 PS25, Line 91: ASSERT_ERROR_MSG(key.Encrypt(scratch.data(),buffer_size, scratch.data()), > Please ignore the above message, added. Done. Ack -- To view, visit http://gerrit.cloudera.org:8080/20447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9 Gerrit-Change-Number: 20447 Gerrit-PatchSet: 28 Gerrit-Owner: Pranav Lodha <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Comment-Date: Mon, 25 Nov 2024 18:27:01 +0000 Gerrit-HasComments: Yes
