hudi-agent commented on code in PR #19337:
URL: https://github.com/apache/hudi/pull/19337#discussion_r3624524080
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/internal/utils/AvroSchemaEvolutionUtils.java:
##########
@@ -78,7 +78,19 @@ public static InternalSchema reconcileSchema(HoodieSchema
incomingSchema, Intern
.stream()
.filter(f -> colNamesFromOldSchema.contains(f) &&
!inComingInternalSchema.findType(f).equals(oldTableSchema.findType(f)))
.collect(Collectors.toList());
- if (colNamesFromIncoming.size() == colNamesFromOldSchema.size() &&
diffFromOldSchema.size() == 0 && typeChangeColumns.isEmpty()) {
+ // check columns the incoming schema relaxed from required to nullable.
Since the result is built from
+ // oldTableSchema (to preserve column order/ids and to null-fill missing
columns), an existing column
+ // whose incoming counterpart became nullable would otherwise silently
keep the table's REQUIRED
+ // nullability, blocking a valid required -> nullable evolution. We only
ever relax (never tighten).
+ List<String> nullabilityRelaxColumns = colNamesFromIncoming
Review Comment:
🤖 nit: `nullabilityRelaxColumns` reads a bit oddly because `relax` is a verb
— could you rename it to `relaxedToNullableColumns` (or
`nullabilityRelaxedColumns`) to make it clear this is a list of columns *that
have been* relaxed, matching the past-tense-adjective pattern of
`typeChangeColumns` nearby?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
