huan233usc commented on code in PR #793:
URL: https://github.com/apache/iceberg-cpp/pull/793#discussion_r3509393243
##########
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:
Thanks for catching this -- yes it stored only the canonical full_name but
callers use the short name. Fixed by also indexing the short name (matching how
the schema name index handles map value / list element paths), with a
regression test for requiring a defaulted nested column by short name in the
same transaction.
It seems that same bug exists in Java; working on a fixed
apache/iceberg#17034.
##########
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:
Thanks for catching this -- yes it stored only the canonical full_name but
callers use the short name. Fixed by also indexing the short name (matching how
the schema name index handles map value / list element paths), with a
regression test for requiring a defaulted nested column by short name in the
same transaction.
It seems that same bug exists in Java; working on a fix apache/iceberg#17034.
--
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]
