You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ap...@apache.org on 2020/05/20 10:04:16 UTC
[arrow] branch master updated: ARROW-8871: [C++] Fix Gandiva for
value_parsing.h refactor
This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new e570627 ARROW-8871: [C++] Fix Gandiva for value_parsing.h refactor
e570627 is described below
commit e5706270669f48fa1c16acaab2b2f7722b72beaf
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Wed May 20 12:03:47 2020 +0200
ARROW-8871: [C++] Fix Gandiva for value_parsing.h refactor
Closes #7232 from pitrou/ARROW-8871-gandiva-value-parsing
Authored-by: Antoine Pitrou <an...@python.org>
Signed-off-by: Antoine Pitrou <an...@python.org>
---
cpp/src/arrow/scalar.cc | 3 +--
cpp/src/arrow/util/value_parsing.cc | 7 ++++---
cpp/src/arrow/util/value_parsing.h | 6 +++---
cpp/src/gandiva/execution_context.h | 14 ++++++--------
cpp/src/gandiva/precompiled/arithmetic_ops.cc | 5 ++---
5 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc
index 690c7ba..c3e2811 100644
--- a/cpp/src/arrow/scalar.cc
+++ b/cpp/src/arrow/scalar.cc
@@ -220,9 +220,8 @@ std::string Scalar::ToString() const {
}
struct ScalarParseImpl {
- // XXX Use of detail here not ideal
template <typename T,
- typename Value = typename internal::detail::StringConverter<T>::value_type>
+ typename Value = typename internal::StringConverter<T>::value_type>
Status Visit(const T& t) {
Value value;
if (!internal::ParseValue<T>(s_.data(), s_.size(), &value)) {
diff --git a/cpp/src/arrow/util/value_parsing.cc b/cpp/src/arrow/util/value_parsing.cc
index 008590d..91b50de 100644
--- a/cpp/src/arrow/util/value_parsing.cc
+++ b/cpp/src/arrow/util/value_parsing.cc
@@ -24,7 +24,8 @@
namespace arrow {
namespace internal {
-namespace detail {
+
+namespace {
struct StringToFloatConverterImpl {
StringToFloatConverterImpl()
@@ -46,6 +47,8 @@ struct StringToFloatConverterImpl {
static const StringToFloatConverterImpl g_string_to_float;
+} // namespace
+
bool StringToFloat(const char* s, size_t length, float* out) {
int processed_length;
float v;
@@ -79,8 +82,6 @@ bool StringToFloat(const char* s, size_t length, double* out) {
return true;
}
-} // namespace detail
-
// ----------------------------------------------------------------------
// strptime-like parsing
diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h
index 1f6c80e..f0f5214 100644
--- a/cpp/src/arrow/util/value_parsing.h
+++ b/cpp/src/arrow/util/value_parsing.h
@@ -55,8 +55,6 @@ class ARROW_EXPORT TimestampParser {
namespace internal {
-namespace detail {
-
template <typename ARROW_TYPE, typename Enable = void>
struct StringConverter;
@@ -347,6 +345,8 @@ struct StringConverter<Int64Type> : public StringToSignedIntConverterMixin<Int64
using StringToSignedIntConverterMixin<Int64Type>::StringToSignedIntConverterMixin;
};
+namespace detail {
+
// Inline-able ISO-8601 parser
using ts_type = TimestampType::c_type;
@@ -458,7 +458,7 @@ static inline bool ParseHH_MM_SS(const char* s, std::chrono::duration<ts_type>*
template <typename T, typename ParseContext = void>
inline bool ParseValue(const char* s, size_t length, typename T::c_type* out,
const ParseContext* ctx = NULLPTR) {
- return detail::StringConverter<T>::Convert(s, length, out);
+ return StringConverter<T>::Convert(s, length, out);
}
static inline bool ParseTimestampISO8601(const char* s, size_t length,
diff --git a/cpp/src/gandiva/execution_context.h b/cpp/src/gandiva/execution_context.h
index 0844c08..1e9ae20 100644
--- a/cpp/src/gandiva/execution_context.h
+++ b/cpp/src/gandiva/execution_context.h
@@ -19,13 +19,16 @@
#include <memory>
#include <string>
-#include "arrow/util/parsing.h"
+#include "arrow/util/value_parsing.h"
#include "gandiva/simple_arena.h"
namespace gandiva {
/// Execution context during llvm evaluation
class ExecutionContext {
+ using FloatConverter = arrow::internal::StringConverter<arrow::FloatType>;
+ using DoubleConverter = arrow::internal::StringConverter<arrow::DoubleType>;
+
public:
explicit ExecutionContext(arrow::MemoryPool* pool = arrow::default_memory_pool())
: arena_(pool) {}
@@ -48,21 +51,16 @@ class ExecutionContext {
}
bool parse_float(const char* data, int32_t len, float* val) {
- return float_converter_(data, len, val);
+ return FloatConverter::Convert(data, len, val);
}
bool parse_double(const char* data, int32_t len, double* val) {
- return double_converter_(data, len, val);
+ return DoubleConverter::Convert(data, len, val);
}
private:
std::string error_msg_;
SimpleArena arena_;
-
- // Keeping float_converters as part of execution context since they have non-trivial
- // construction cost
- arrow::internal::StringConverter<arrow::FloatType> float_converter_;
- arrow::internal::StringConverter<arrow::DoubleType> double_converter_;
};
} // namespace gandiva
diff --git a/cpp/src/gandiva/precompiled/arithmetic_ops.cc b/cpp/src/gandiva/precompiled/arithmetic_ops.cc
index 443db24..54facdf 100644
--- a/cpp/src/gandiva/precompiled/arithmetic_ops.cc
+++ b/cpp/src/gandiva/precompiled/arithmetic_ops.cc
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-#include "arrow/util/parsing.h"
+#include "arrow/util/value_parsing.h"
extern "C" {
@@ -239,8 +239,7 @@ DIV_FLOAT(float64)
gdv_##OUT_TYPE cast##TYPE_NAME##_utf8(int64_t context, const char* data, \
int32_t len) { \
gdv_##OUT_TYPE val; \
- arrow::internal::StringConverter<ARROW_TYPE> converter_; \
- if (!converter_(data, len, &val)) { \
+ if (!arrow::internal::StringConverter<ARROW_TYPE>::Convert(data, len, &val)) { \
gdv_fn_context_set_error_msg(context, \
"Failed parsing the string to required format"); \
} \