Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20447 )
Change subject: IMPALA-12418: Include crypto functions supported by Hive ...................................................................... Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/20447/6/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/20447/6/be/src/exprs/string-functions-ir.cc@160 PS6, Line 160: "space() result", > These lines should still be indented 4 spaces. Done http://gerrit.cloudera.org:8080/#/c/20447/6/be/src/exprs/string-functions-ir.cc@1347 PS6, Line 1347: StringValue::FromStringVal(key), &result)) { > This line should still be indented. Done http://gerrit.cloudera.org:8080/#/c/20447/6/be/src/exprs/string-functions-ir.cc@1826 PS6, Line 1826: static int encrypt_ecb(FunctionContext* ctx, const unsigned char* input, int input_len, > I don't think we can do that as all of the functions that follow uses an un Did you look at the example at the bottom of the linked page? That error is because you declare an EVP_CIPHER_CTX* and never assign anything to it. The example uses a value EVP_CIPHER_CTX ctx; EVP_CIPHER_CTX_init(&ctx); EVP_EncryptInit_ex(&ctx, EVP_idea_cbc(), NULL, key, iv); ... http://gerrit.cloudera.org:8080/#/c/20447/6/be/src/exprs/string-functions-ir.cc@1865 PS6, Line 1865: EVP_CIPHER_CTX_free(cipher_ctx); > Added at 1905 Ack http://gerrit.cloudera.org:8080/#/c/20447/6/be/src/exprs/string-functions-ir.cc@1938 PS6, Line 1938: cipher = EVP_aes_256_ecb(); > I think this is the only place where we're checking for output_len. When encrypt_ecb encounters an error, it calls SetError and returns -1. So maybe we should only add SetError if ret_size is not -1? http://gerrit.cloudera.org:8080/#/c/20447/8/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/20447/8/be/src/exprs/string-functions-ir.cc@1862 PS8, Line 1862: DCHECK(EVP_CIPHER_block_size(cipher) == 16); nit: could be a DCHECK_EQ. -- 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: 8 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Tue, 23 Jan 2024 18:41:50 +0000 Gerrit-HasComments: Yes
