You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/11/02 06:10:47 UTC
[GitHub] [arrow] kou commented on a change in pull request #11587: ARROW-14530: [GLib] Return error for invalid decimal string
kou commented on a change in pull request #11587:
URL: https://github.com/apache/arrow/pull/11587#discussion_r740743773
##########
File path: c_glib/arrow-glib/decimal.cpp
##########
@@ -60,11 +60,18 @@ struct DecimalConverter<arrow::Decimal256> {
template <typename Decimal>
typename DecimalConverter<Decimal>::GArrowType *
-garrow_decimal_new_string(const gchar *data)
-{
- auto arrow_decimal = std::make_shared<Decimal>(data);
- DecimalConverter<Decimal> converter;
- return converter.new_raw(&arrow_decimal);
+garrow_decimal_new_string(const gchar *data,
+ GError **error,
+ const gchar *tag)
+{
+ auto arrow_decimal_result = Decimal::FromString(data);
+ if (garrow::check(error, arrow_decimal_result, tag)) {
+ auto arrow_decimal = std::make_shared<Decimal>(*arrow_decimal_result);
+ DecimalConverter<Decimal> converter;
+ return converter.new_raw(&arrow_decimal);
+ } else {
+ return nullptr;
+ }
Review comment:
Good catch.
Generally, I use `NULL` for GLib (C) related API. This should be this case because we just returns the return value of this template function in GLib related API. I'll change to use `NULL`.
##########
File path: c_glib/arrow-glib/decimal.cpp
##########
@@ -60,11 +60,18 @@ struct DecimalConverter<arrow::Decimal256> {
template <typename Decimal>
typename DecimalConverter<Decimal>::GArrowType *
-garrow_decimal_new_string(const gchar *data)
-{
- auto arrow_decimal = std::make_shared<Decimal>(data);
- DecimalConverter<Decimal> converter;
- return converter.new_raw(&arrow_decimal);
+garrow_decimal_new_string(const gchar *data,
+ GError **error,
+ const gchar *tag)
+{
+ auto arrow_decimal_result = Decimal::FromString(data);
+ if (garrow::check(error, arrow_decimal_result, tag)) {
+ auto arrow_decimal = std::make_shared<Decimal>(*arrow_decimal_result);
+ DecimalConverter<Decimal> converter;
+ return converter.new_raw(&arrow_decimal);
+ } else {
+ return nullptr;
Review comment:
```suggestion
return NULL;
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org