Copilot commented on code in PR #2765:
URL: https://github.com/apache/fluss/pull/2765#discussion_r2872407475
##########
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/tiering/FlussRecordAsPaimonRow.java:
##########
@@ -49,6 +51,7 @@ public FlussRecordAsPaimonRow(int bucket, RowType
tableTowType) {
}
public void setFlussRecord(LogRecord logRecord) {
+ checkNotNull(logRecord, "logRecord must not be null");
Review Comment:
Minor consistency: the new checkNotNull message doesn't end with a period,
while the other (new) precondition messages in this class do. Consider making
this message consistent (e.g., punctuation) to keep error output uniform.
```suggestion
checkNotNull(logRecord, "logRecord must not be null.");
```
##########
fluss-lake/fluss-lake-paimon/src/main/java/org/apache/fluss/lake/paimon/tiering/FlussRecordAsPaimonRow.java:
##########
@@ -74,11 +77,13 @@ public int getFieldCount() {
@Override
public RowKind getRowKind() {
+ checkState(logRecord != null, "setFlussRecord() must be called before
accessing the row.");
return toRowKind(logRecord.getChangeType());
Review Comment:
The new defensive preconditions (e.g., this checkState) change the failure
mode from an opaque NPE to an IllegalStateException. There are existing unit
tests for this class, but none currently assert the uninitialized-access
behavior (calling accessors before setFlussRecord()) or setFlussRecord(null).
Please add tests to lock in the exception type + message for these cases.
--
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]
