Merge two Schemas #263
Merge two Schemas #263
Conversation
eddyxu
added
c++
eddyxu
force-pushed
the
lei/merge_schema
branch
from
82c9668 to
90ec4c7
Compare
cpp/src/lance/format/schema.cc
Outdated
| auto self_type = type(); | ||
| if (self_type->id() != arrow_field.type()->id()) { | ||
| return ::arrow::Status::Invalid( | ||
| "Can not merge two fields: ", self_type->ToString(), "!=", arrow_field.type()->ToString()); |
There was a problem hiding this comment.
nit: change the message to indicate that it's trying to merge two different types
cpp/src/lance/format/schema.cc
Outdated
| auto new_field = Copy(true); | ||
| if (::arrow::is_list_like(self_type->id())) { | ||
| auto list_type = std::dynamic_pointer_cast<::arrow::ListType>(arrow_field.type()); | ||
| return children_[0]->Merge(*list_type->value_field()); |
There was a problem hiding this comment.
I don't quite understand this, if we're merging two list fields, then should the return value be a list type of the merge of the two value types?
There was a problem hiding this comment.
this is the case for merging list<struct<....>>, so the struct type is nested value type for a list.
There was a problem hiding this comment.
I just see is_list_like, but how do we know it's a struct inside the list field?
| } else { | ||
| ARROW_ASSIGN_OR_RAISE(auto new_child_field, child->Merge(*arrow_child)); | ||
| new_field->children_.emplace_back(new_child_field); | ||
| } |
There was a problem hiding this comment.
what about children of this struct field that's not in the arrow_field?
There was a problem hiding this comment.
new_field has the children copied here https://github.com/eto-ai/lance/pull/263/files/90ec4c778ad42bf0f54d1abb0b8aef09e45bdd22#diff-0459f40916915b924d3c6aa11554911c9f00fcded2c0361b85f215f6623cce58R398.
It seems to a bug that the common child between two fields will be insert twice tho. Lemme fix it.
eddyxu
force-pushed
the
lei/merge_schema
branch
from
00e505e to
9333478
Compare
eddyxu
force-pushed
the
lei/merge_schema
branch
from
9333478 to
a519f56
Compare
Merge two nested schemas.