github-actions[bot] commented on code in PR #67334:
URL: https://github.com/apache/doris/pull/67334#discussion_r3901695225
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnToProtobuf.java:
##########
@@ -77,6 +77,9 @@ public static OlapFile.ColumnPB toPb(Column column,
Set<String> bfColumns, List<
if (column.getDefaultValue() != null) {
builder.setDefaultValue(ByteString.copyFrom(column.getDefaultValue().getBytes()));
}
+ if (column.getDefaultValueExprDef() != null) {
Review Comment:
[P1] Keep the frozen cloud backfill literal in default_value
For a schema-change Column, getDefaultValue() is CURRENT_TIMESTAMP(6) while
getRealDefaultValue() is the DDL-time literal. createTabletMetaBuilder uses
this protobuf for cloud shadow tablets, and SchemaChangeJob later parses
new_column.default_value() as a literal. This stores the expression in both
fields; DATETIMEV2 conversion swallows the invalid literal and substitutes
MIN_DATETIME_V2, so historical rows get corrupted instead of receiving the
DDL-time value. Please mirror ColumnToThrift for default_value and assert the
frozen value in the protobuf test, with cloud heavy-schema-change coverage.
##########
be/src/storage/partial_update_info.cpp:
##########
@@ -269,12 +269,13 @@ void
PartialUpdateInfo::_generate_default_values_for_missing_cids(
for (unsigned int cur_cid : missing_cids) {
const auto& column = tablet_schema.column(cur_cid);
if (column.has_default_value()) {
+ const auto& default_value_expr =
column.default_value_for_partial_update();
Review Comment:
[P1] Evaluate legacy DATETIME before preferring the expression
FE still permits CURRENT_TIMESTAMP on DATETIME, and those V1 schemas remain
reachable. Before this change the frozen timestamp literal parsed successfully;
now default_value_for_partial_update() selects CURRENT_TIMESTAMP, but the
branches below handle only DATETIMEV2/TIMESTAMPTZ. The V1 literal SerDe rejects
the expression, so a NOT NULL partial update fails and a nullable column can
silently become NULL. Please evaluate OLAP_FIELD_TYPE_DATETIME too and add
legacy fixed/flexible coverage.
--
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]
