felipecrv commented on code in PR #43485:
URL: https://github.com/apache/arrow/pull/43485#discussion_r1697495817
##########
cpp/src/arrow/array/concatenate.cc:
##########
@@ -565,10 +565,25 @@ class ConcatenateImpl {
}
Status Visit(const StructType& s) {
+ std::shared_ptr<StructType> suggested = nullptr;
for (int i = 0; i < s.num_fields(); ++i) {
ARROW_ASSIGN_OR_RAISE(auto child_data, ChildData(i));
- RETURN_NOT_OK(ConcatenateImpl(child_data, pool_)
- .Concatenate(&out_->child_data[i], /*hints=*/nullptr));
+ ErrorHints child_hints;
Review Comment:
rename to `field_error_hints` or just `error_hints` because these are "hints
about error" states.
##########
cpp/src/arrow/array/concatenate.cc:
##########
@@ -565,10 +565,25 @@ class ConcatenateImpl {
}
Status Visit(const StructType& s) {
+ std::shared_ptr<StructType> suggested = nullptr;
for (int i = 0; i < s.num_fields(); ++i) {
ARROW_ASSIGN_OR_RAISE(auto child_data, ChildData(i));
- RETURN_NOT_OK(ConcatenateImpl(child_data, pool_)
- .Concatenate(&out_->child_data[i], /*hints=*/nullptr));
+ ErrorHints child_hints;
+ auto status = ConcatenateImpl(child_data, pool_)
+ .Concatenate(&out_->child_data[i], &child_hints);
+ if (!status.ok() && child_hints.suggested_cast) {
+ ARROW_ASSIGN_OR_RAISE(
+ suggested,
+ suggested
+ ? suggested->SetField(
+ i,
s.field(i)->WithType(std::move(child_hints.suggested_cast)))
+ : s.SetField(
+ i,
s.field(i)->WithType(std::move(child_hints.suggested_cast))));
+ }
+ }
+ if (suggested) {
+ suggested_cast_ = std::move(suggested);
+ return OffsetOverflowStatus();
Review Comment:
```suggestion
if (!suggested_field_casts.empty()) {
suggested_cast_ = struct_(suggested_field_casts);
return OffsetOverflowStatus();
}
```
##########
cpp/src/arrow/array/concatenate.cc:
##########
@@ -565,10 +565,25 @@ class ConcatenateImpl {
}
Status Visit(const StructType& s) {
+ std::shared_ptr<StructType> suggested = nullptr;
for (int i = 0; i < s.num_fields(); ++i) {
ARROW_ASSIGN_OR_RAISE(auto child_data, ChildData(i));
- RETURN_NOT_OK(ConcatenateImpl(child_data, pool_)
- .Concatenate(&out_->child_data[i], /*hints=*/nullptr));
+ ErrorHints child_hints;
+ auto status = ConcatenateImpl(child_data, pool_)
+ .Concatenate(&out_->child_data[i], &child_hints);
+ if (!status.ok() && child_hints.suggested_cast) {
+ ARROW_ASSIGN_OR_RAISE(
+ suggested,
+ suggested
+ ? suggested->SetField(
+ i,
s.field(i)->WithType(std::move(child_hints.suggested_cast)))
+ : s.SetField(
+ i,
s.field(i)->WithType(std::move(child_hints.suggested_cast))));
+ }
+ }
Review Comment:
```suggestion
if (!status.ok()) {
if (child_hints.suggested_cast) {
// There is a hint that the error can be fixed by casting the
field.
if (suggested_field_casts.empty()) {
suggested_field_casts = s.fields();
}
suggested_field_casts[i] = std::move(child_hints.suggested_cast);
} else {
// No hint, we should abort by returning the bad status.
return status;
}
}
```
##########
cpp/src/arrow/array/concatenate.cc:
##########
@@ -565,10 +565,25 @@ class ConcatenateImpl {
}
Status Visit(const StructType& s) {
+ std::shared_ptr<StructType> suggested = nullptr;
Review Comment:
```suggestion
// Populated if the concatenation of some field fails due to overflow.
// Then it's used in the end to produce the StructType suggestion.
std::vector<std::shared_ptr<Field>> suggested_field_casts;
```
--
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]
