Reranko05 commented on PR #49660: URL: https://github.com/apache/arrow/pull/49660#issuecomment-4215376028
Hi @kou, I re-checked the usages of base64_decode() across the codebase. Besides tests, it is used in multiple modules: Flight: ``` cpp/src/arrow/flight/flight_test.cc:623: std::stringstream decoded_stream(arrow::util::base64_decode(encoded_credentials)); ``` Gandiva: ``` cpp/src/gandiva/gdv_function_stubs.cc:272: std::string decoded_str = arrow::util::base64_decode(std::string_view(in, in_len)); ``` Parquet: ``` cpp/src/parquet/arrow/fuzz_internal.cc:87: ::arrow::util::base64_decode(key_id.substr(kInlineKeyPrefix.length()))); cpp/src/parquet/arrow/schema.cc:956: auto decoded = ::arrow::util::base64_decode(metadata->value(schema_index)); cpp/src/parquet/encryption/file_key_unwrapper.cc:125: std::string aad = ::arrow::util::base64_decode(encoded_kek_id); cpp/src/parquet/encryption/key_toolkit_internal.cc:55: std::string encrypted_key = ::arrow::util::base64_decode(encoded_encrypted_key); ``` These call sites currently expect a std::string (e.g., direct assignment or usage in std::stringstream). After updating the API to return arrow::Result<std::string>, CI is failing due to these usages. So the impact appears broader than I initially estimated and spans multiple components. Would you prefer that I: - Update all affected call sites in this PR to handle `arrow::Result<std::string>`, or - Introduce a new checked API while keeping the existing `base64_decode` unchanged? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
