manuzhang commented on code in PR #793:
URL: https://github.com/apache/iceberg-cpp/pull/793#discussion_r3504704865
##########
src/iceberg/update/update_schema.cc:
##########
@@ -474,9 +555,10 @@ UpdateSchema&
UpdateSchema::UpdateColumnRequirementInternal(std::string_view nam
return *this;
}
- // TODO(GuotaoYu): support added column with default value
- // bool is_defaulted_add = IsAdded(name) && field.initial_default() != null;
- bool is_defaulted_add = false;
+ // A column added in this update with a default value can be made required:
rows
+ // written before the change read the initial-default instead of null.
+ bool is_defaulted_add =
added_name_to_id_.contains(CaseSensitivityAwareName(name)) &&
Review Comment:
For map/list nested adds, `AddColumnInternal` stores the new field in
`added_name_to_id_` using the canonical internal path:
```
added_name_to_id_[CaseSensitivityAwareName(full_name)] = new_id;
```
where full_name becomes something like:
```
locations.value.alt
points.element.z
```
But `RequireColumn` receives and checks the user-facing shorthand path:
```
added_name_to_id_.contains(CaseSensitivityAwareName(name))
```
where name is typically:
```
locations.alt
points.z
```
So the lookup misses, even though it refers to the same added column
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
