jerryshao commented on code in PR #12551:
URL: https://github.com/apache/gravitino/pull/12551#discussion_r3853146560
##########
core/src/main/java/org/apache/gravitino/storage/relational/po/TablePO.java:
##########
@@ -120,6 +120,28 @@ private Builder() {
tablePO = new TablePO();
}
+ // Copies every column of the source row. When a field is added to TablePO
above, add it here
+ // too, otherwise callers that copy a row would silently blank it.
+ private Builder(TablePO source) {
Review Comment:
**Simplification/maintenance risk: hand-rolled full-field copy constructor.**
This copy constructor duplicates every `TablePO` field by hand, guarded only
by the comment "When a field is added to TablePO above, add it here too,
otherwise callers that copy a row would silently blank it." That's a
maintenance trap enforced by convention rather than the compiler — a future
field addition that misses this constructor fails silently (no compile error,
no test failure unless the new field happens to be asserted). No shared "copy a
PO" utility exists elsewhere in the codebase (e.g. `SchemaPO`'s equivalent need
is met via a narrower `copySchemaPOWithName`-style static builder), so this
exact hazard is likely to recur for the next PO that needs copy semantics.
--
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]
