stevenzwu commented on code in PR #7517:
URL: https://github.com/apache/iceberg/pull/7517#discussion_r1188912538
##########
api/src/main/java/org/apache/iceberg/util/StructProjection.java:
##########
@@ -174,6 +174,7 @@ private StructProjection(StructType structType, StructType
projection, boolean a
}
public StructProjection wrap(StructLike newStruct) {
+ Preconditions.checkState(newStruct != null, "Invalid root struct for
wrapping: null");
Review Comment:
I feel the removal of if-check and adding Preconditions check should go
hand-to-hand. Either we do both or don't change it all. Otherwise, we can get
NPE for wrapping root null object.
One Preconditions check shouldn't cause performance degradation. Overall, we
removed one if check and added a new check in Preconditions.
Here are a few usages on the `StructProjection#wrap(StructLike)`
* core: transform on `CloseableIterable<StructLike>` for manifest or file
entries
* data: delete filter on record
* flink: always wrap non-null `RowDataWrapper` object for
EqualityFieldKeySelector
* spark: always wrap non-null `InternalRowWrapper` object for wrapping
partition data
@RussellSpitzer @rdblue do you see any potential null root values for the
core and data module usage of wrap.
--
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]
