Changes made by argmarco@ to resolve issues with the merge to main. - #225
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or See also: |
|
|
||
| #pragma once | ||
|
|
||
| #include <cstddef> |
There was a problem hiding this comment.
Do we really need these two imports just to switch to size_t?
There was a problem hiding this comment.
Good catch. Let me double check.
| : ciphertext_data_(std::move(data)), success_(success), | ||
| error_message_(std::move(error_msg)), error_fields_(std::move(error_fields)) {} | ||
| error_message_(std::move(error_msg)), error_fields_(std::move(error_fields)), | ||
| metadata_(std::move(metadata)) {} |
There was a problem hiding this comment.
Why was this needed? I thought only size_t issues were coming up.
There was a problem hiding this comment.
One of the earlier commits in this PR was to update the DBPA interface. This change is part of that (DBPA Result objets have a new virtual method that I was forced to implement here). It has nothing to do with size_t errors.
| const std::map<std::string, std::string>& GetLastEncryptEncodingAttrs() const { | ||
| return last_encrypt_encoding_attrs_; | ||
| } | ||
| const std::map<std::string, std::string>& GetLastDecryptEncodingAttrs() const { |
There was a problem hiding this comment.
Same question with the failures
There was a problem hiding this comment.
This is part of the updates to DBPA.
| std::string column_name = "employee_name"; | ||
| std::string key_id = "employee_name_key"; | ||
| Type::type data_type = Type::BYTE_ARRAY; | ||
| Compression::type compression_type = Compression::UNCOMPRESSED; |
There was a problem hiding this comment.
basically, regression testing. We added new code in external_dbpa_encryption.cc to ensure that no memcpy happened with zero-sized arrays (one of the things that the Sanitizer complained about). Decided to add a test to validate the new behavior.
… from the compiler
… a regression test
…e sanitizer to be unhappy
5fe2e75 to
b63ada7
Compare
2 participants
Changes made by argmarco@ to resolve issues with the merge to main.