manuzhang commented on code in PR #793:
URL: https://github.com/apache/iceberg-cpp/pull/793#discussion_r3510215971
##########
src/iceberg/update/update_schema.cc:
##########
@@ -420,9 +459,52 @@ UpdateSchema&
UpdateSchema::UpdateColumnDoc(std::string_view name,
return *this;
}
- updates_[field_id] =
- std::make_shared<SchemaField>(field.field_id(), field.name(),
field.type(),
- field.optional(), std::string(new_doc));
+ updates_[field_id] = std::make_shared<SchemaField>(
+ field.field_id(), field.name(), field.type(), field.optional(), new_doc,
+ field.initial_default(), field.write_default());
+
+ return *this;
+}
+
+UpdateSchema& UpdateSchema::UpdateColumnDefault(std::string_view name,
+ std::optional<Literal>
new_default) {
+ ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto field_opt, FindFieldForUpdate(name));
+ ICEBERG_BUILDER_CHECK(field_opt.has_value(), "Cannot update missing column:
{}", name);
+
+ const auto& field = field_opt->get();
+ int32_t field_id = field.field_id();
+
+ ICEBERG_BUILDER_CHECK(!deletes_.contains(field_id),
+ "Cannot update a column that will be deleted: {}",
field.name());
+
+ // Only the write-default is replaced; the initial-default is fixed when the
column is
+ // added and is preserved here.
+ auto rebuild = [&](std::shared_ptr<const Literal> write_default) {
+ return std::make_shared<SchemaField>(
+ field.field_id(), field.name(), field.type(), field.optional(),
field.doc(),
+ field.initial_default(), std::move(write_default));
+ };
+
+ // An empty default clears the column's write-default.
+ if (!new_default.has_value()) {
+ updates_[field_id] = rebuild(nullptr);
+ return *this;
+ }
+
+ ICEBERG_BUILDER_CHECK(field.type()->is_primitive(),
+ "Invalid default value for {}: {} (must be null)",
*field.type(),
+ *new_default);
+ ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR(
+ Literal typed_default,
+
new_default->CastTo(internal::checked_pointer_cast<PrimitiveType>(field.type())),
Review Comment:
Could we share the decimal default promotion logic used above in
`UpdateColumn` here too? `Literal::CastTo` only returns decimal values when
source and target `DecimalType` are exactly equal; it does not handle
same-scale precision widening. That means adding `decimal(18,2)` with
`Literal::Decimal(..., 9, 2)` is rejected even though this is the same allowed
promotion handled above. The same issue appears in `AddColumnInternal`.
--
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]
