Skip to content

Commit

Permalink
use to_string instead of fmt::format
Browse files Browse the repository at this point in the history
  • Loading branch information
muhammadhamzasajjad committed May 16, 2024
1 parent 5486084 commit 18beb3f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 56 deletions.
2 changes: 1 addition & 1 deletion cpp/arcticdb/entity/type_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace entity {
);

inline std::string get_user_friendly_type_string(const entity::TypeDescriptor& type) {
return is_sequence_type(type.data_type()) ? "TD<type=STRING, dim=0>" : fmt::format("{}", type);
return is_sequence_type(type.data_type()) ? fmt::format("TD<type=STRING, dim={}>", type.dimension_) : fmt::format("{}", type);
}

}
33 changes: 10 additions & 23 deletions cpp/arcticdb/pipeline/value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ struct Value {
*str_data() = data;
}

template<typename RawType>
std::string to_string() const {
if (has_sequence_type()) {
return "\"" + std::string(*str_data(), len()) + "\"";
}
else {
return fmt::format("{}", get<RawType>());
}
}

~Value() {
if(has_sequence_type())
delete[] *str_data();
Expand Down Expand Up @@ -209,26 +219,3 @@ inline std::optional<std::string> ascii_to_padded_utf32(std::string_view str, si
}

} //namespace arcticdb

namespace fmt {
template<>
struct formatter<arcticdb::Value> {
template<typename ParseContext>
constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }

template<typename FormatContext>
auto format(const arcticdb::Value& value, FormatContext& ctx) const {
if (is_sequence_type(value.type().data_type())) {
fmt::format_to(ctx.out(), "\"{}\"", std::string(*value.str_data(), value.len()));
}
else {
arcticdb::entity::details::visit_type(value.data_type_, [&](auto val_tag) {
using type_info = arcticdb::entity::ScalarTypeInfo<decltype(val_tag)>;
fmt::format_to(ctx.out(), "{}", value.get<typename type_info::RawType>());
});
}

return ctx.out();
}
};
}
38 changes: 19 additions & 19 deletions cpp/arcticdb/processing/operation_dispatch_binary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ VariantData binary_boolean(EmptyResult, EmptyResult, OperationType operation);
// commutative, however if that were to change we would need the full set
VariantData visit_binary_boolean(const VariantData& left, const VariantData& right, OperationType operation);

template <typename Func>
inline std::string binary_operation_column_name(std::string_view left_column, Func&& func, std::string_view right_column) {
return fmt::format("({} {} {})", left_column, func, right_column);
}

template <typename Func>
inline std::string binary_operation_with_types_to_string(std::string_view left, const TypeDescriptor& type_left, Func&& func,
std::string_view right, const TypeDescriptor& type_right,
bool arguments_reversed = false) {
if (arguments_reversed) {
return fmt::format("{} ({}) {} {} ({})", right, get_user_friendly_type_string(type_right), func, left, get_user_friendly_type_string(type_left));
}
return fmt::format("{} ({}) {} {} ({})", left, get_user_friendly_type_string(type_left), func, right, get_user_friendly_type_string(type_right));
}

template <typename Func>
VariantData binary_membership(const ColumnWithStrings& column_with_strings, ValueSet& value_set, Func&& func) {
if (is_empty_type(column_with_strings.column_->type().data_type())) {
Expand Down Expand Up @@ -252,7 +267,7 @@ VariantData binary_comparator(const ColumnWithStrings& column_with_strings, cons
column_with_strings.column_name_,
column_with_strings.column_->type(),
func,
fmt::format("{}", val),
val.to_string<typename val_type_info::RawType>(),
val.type(),
arguments_reversed));
}
Expand Down Expand Up @@ -292,21 +307,6 @@ VariantData visit_binary_comparator(const VariantData& left, const VariantData&
}, left, right);
}

template <typename Func>
inline std::string binary_operation_column_name(std::string_view left_column, Func&& func, std::string_view right_column) {
return fmt::format("({} {} {})", left_column, func, right_column);
}

template <typename Func>
inline std::string binary_operation_with_types_to_string(std::string_view left, const TypeDescriptor& type_left, Func&& func,
std::string_view right, const TypeDescriptor& type_right,
bool arguments_reversed = false) {
if (arguments_reversed) {
return fmt::format("{} ({}) {} {} ({})", right, get_user_friendly_type_string(type_right), func, left, get_user_friendly_type_string(type_left));
}
return fmt::format("{} ({}) {} {} ({})", left, get_user_friendly_type_string(type_left), func, right, get_user_friendly_type_string(type_right));
}

template <typename Func>
VariantData binary_operator(const Value& left, const Value& right, Func&& func) {
auto output_value = std::make_unique<Value>();
Expand All @@ -318,10 +318,10 @@ VariantData binary_operator(const Value& left, const Value& right, Func&& func)
if constexpr(!is_numeric_type(left_type_info::data_type) || !is_numeric_type(right_type_info::data_type)) {
user_input::raise<ErrorCode::E_INVALID_USER_ARGUMENT>("Non-numeric type provided to binary operation: {}",
binary_operation_with_types_to_string(
fmt::format("{}", left),
left.to_string<typename left_type_info::RawType>(),
left.type(),
func,
fmt::format("{}", right),
right.to_string<typename right_type_info::RawType>(),
right.type()));
}
auto right_value = *reinterpret_cast<const typename right_type_info::RawType*>(right.data_);
Expand Down Expand Up @@ -388,7 +388,7 @@ VariantData binary_operator(const ColumnWithStrings& col, const Value& val, Func
col.column_name_,
col.column_->type(),
func,
fmt::format("{}", val),
val.to_string<typename val_type_info::RawType>(),
val.type(),
arguments_reversed));
}
Expand Down
26 changes: 13 additions & 13 deletions cpp/arcticdb/processing/operation_dispatch_unary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ VariantData unary_boolean(FullResult, OperationType operation);

VariantData visit_unary_boolean(const VariantData& left, OperationType operation);

template <typename Func>
inline std::string unary_operation_to_string(Func&& func, std::string_view operand_str) {
return fmt::format("{}({})", func, operand_str);
}

template <typename Func>
VariantData unary_operator(const Value& val, Func&& func) {
auto output = std::make_unique<Value>();
Expand All @@ -45,7 +50,7 @@ VariantData unary_operator(const Value& val, Func&& func) {
using type_info = ScalarTypeInfo<decltype(val_tag)>;
if constexpr (!is_numeric_type(type_info::data_type)) {
user_input::raise<ErrorCode::E_INVALID_USER_ARGUMENT>("Cannot perform unary operation {} ({})",
unary_operation_column_name(func, fmt::format("{}", val)),
unary_operation_to_string(func,val.to_string<typename type_info::RawType>()),
get_user_friendly_type_string(val.type()));
}
auto value = *reinterpret_cast<const typename type_info::RawType*>(val.data_);
Expand All @@ -57,17 +62,12 @@ VariantData unary_operator(const Value& val, Func&& func) {
return {std::move(output)};
}

template <typename Func>
inline std::string unary_operation_column_name(Func& func, std::string_view column_name) {
return fmt::format("{}({})", func, column_name);
}

template <typename Func>
VariantData unary_operator(const ColumnWithStrings& col, Func&& func) {
schema::check<ErrorCode::E_UNSUPPORTED_COLUMN_TYPE>(
!is_empty_type(col.column_->type().data_type()),
"Empty column provided to unary operation {} ({})",
unary_operation_column_name(func, col.column_name_),
unary_operation_to_string(func, col.column_name_),
get_user_friendly_type_string(col.column_->type()));
std::unique_ptr<Column> output_column;

Expand All @@ -85,11 +85,11 @@ VariantData unary_operator(const ColumnWithStrings& col, Func&& func) {
});
} else {
user_input::raise<ErrorCode::E_INVALID_USER_ARGUMENT>("Cannot perform unary operation {} ({})",
unary_operation_column_name(func, col.column_name_),
get_user_friendly_type_string(col.column_->type()));
unary_operation_to_string(func, col.column_name_),
get_user_friendly_type_string(col.column_->type()));
}
});
return {ColumnWithStrings(std::move(output_column), unary_operation_column_name(func, col.column_name_))};
return {ColumnWithStrings(std::move(output_column), unary_operation_to_string(func, col.column_name_))};
}

template<typename Func>
Expand Down Expand Up @@ -119,7 +119,7 @@ VariantData unary_comparator(const ColumnWithStrings& col, Func&& func) {
} else if constexpr (std::is_same_v<std::remove_reference_t<Func>, NotNullOperator>) {
return is_empty_type(col.column_->type().data_type()) ? VariantData(EmptyResult{}) : VariantData(FullResult{});
} else {
user_input::raise<ErrorCode::E_INVALID_USER_ARGUMENT>("Unexpected operator passed to unary_comparator");
internal::raise<ErrorCode::E_ASSERTION_FAILURE>("Unexpected operator passed to unary_comparator");
}
}

Expand All @@ -136,7 +136,7 @@ VariantData unary_comparator(const ColumnWithStrings& col, Func&& func) {
return func.template apply<TimeTypeTag>(input_value);
} else {
user_input::raise<ErrorCode::E_INVALID_USER_ARGUMENT>("Cannot perform null check: {} ({})",
unary_operation_column_name(func, col.column_name_),
unary_operation_to_string(func, col.column_name_),
get_user_friendly_type_string(col.column_->type()));
}
});
Expand All @@ -156,7 +156,7 @@ VariantData visit_unary_comparator(const VariantData& left, Func&& func) {
} else if constexpr (std::is_same_v<std::remove_reference_t<Func>, NotNullOperator>) {
return EmptyResult{};
} else {
user_input::raise<ErrorCode::E_INVALID_USER_ARGUMENT>("Unexpected operator passed to visit unary_comparator");
internal::raise<ErrorCode::E_ASSERTION_FAILURE>("Unexpected operator passed to visit unary_comparator");
}
},
[](const auto&) -> VariantData {
Expand Down

0 comments on commit 18beb3f

Please sign in to comment.