zanmato1984 commented on code in PR #41330:
URL: https://github.com/apache/arrow/pull/41330#discussion_r1587894985
##########
cpp/src/gandiva/cache.cc:
##########
@@ -20,26 +20,41 @@
#include "arrow/result.h"
#include "arrow/util/io_util.h"
#include "arrow/util/logging.h"
+#include "arrow/util/value_parsing.h"
namespace gandiva {
-static const size_t DEFAULT_CACHE_SIZE = 5000;
-
-int GetCapacity() {
- size_t capacity = DEFAULT_CACHE_SIZE;
- auto maybe_env_cache_size =
::arrow::internal::GetEnvVar("GANDIVA_CACHE_SIZE");
- if (maybe_env_cache_size.ok()) {
- const auto env_cache_size = *std::move(maybe_env_cache_size);
- if (!env_cache_size.empty()) {
- capacity = std::atol(env_cache_size.c_str());
- if (capacity <= 0) {
- ARROW_LOG(WARNING) << "Invalid cache size provided in
GANDIVA_CACHE_SIZE. "
- << "Using default cache size: " <<
DEFAULT_CACHE_SIZE;
- capacity = DEFAULT_CACHE_SIZE;
- }
- }
+constexpr auto kCacheCapacityEnvVar = "GANDIVA_CACHE_SIZE";
+constexpr auto kDefaultCacheSize = 5000;
+
+namespace internal {
+int GetCacheCapacityFromEnvVar() {
+ auto maybe_env_value = ::arrow::internal::GetEnvVar(kCacheCapacityEnvVar);
+ if (!maybe_env_value.ok()) {
+ return kDefaultCacheSize;
+ }
+ const auto env_value = *std::move(maybe_env_value);
+ if (env_value.empty()) {
+ return kDefaultCacheSize;
+ }
+ int capacity = 0;
+ bool ok = ::arrow::internal::ParseValue<::arrow::Int32Type>(
+ env_value.c_str(), env_value.size(), &capacity);
Review Comment:
Updated the parsing according to the recommendation from
https://github.com/apache/arrow/pull/41335#discussion_r1581117184
--
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]