Modify RmdUtils:getLastUpdateTimestamp to fix class cast exception#2283
Modify RmdUtils:getLastUpdateTimestamp to fix class cast exception#2283suketkar wants to merge 2 commits intolinkedin:mainfrom
Conversation
mynameborat
left a comment
mynameborat
left a comment
There was a problem hiding this comment.
Can you make sure there is enough coverage in unit test? Looks like some of the code coverage checks are failing.
| @Test | ||
| public void testGetLastUpdateTimestampWithCollectionFields() { |
There was a problem hiding this comment.
If there is not an existing test, can you add one for the top level field that contributes to maximum?
| // Handle DELETED_ELEM_TS_FIELD_NAME as a List/Collection | ||
| Object deletedTimestamps = fieldRecord.get(DELETED_ELEM_TS_FIELD_NAME); | ||
| if (deletedTimestamps instanceof List) { | ||
| for (Object ts: (List<?>) deletedTimestamps) { | ||
| lastUpdatedTimestamp = Math.max(lastUpdatedTimestamp, ((Number) ts).longValue()); | ||
| } | ||
| } | ||
| for (long timestamp: (long[]) ((GenericRecord) ((GenericRecord) timestampRecord).get(field.name())) | ||
| .get(ACTIVE_ELEM_TS_FIELD_NAME)) { | ||
| lastUpdatedTimestamp = Math.max(lastUpdatedTimestamp, timestamp); | ||
|
|
||
| // Handle ACTIVE_ELEM_TS_FIELD_NAME as a List/Collection | ||
| Object activeTimestamps = fieldRecord.get(ACTIVE_ELEM_TS_FIELD_NAME); | ||
| if (activeTimestamps instanceof List) { | ||
| for (Object ts: (List<?>) activeTimestamps) { | ||
| lastUpdatedTimestamp = Math.max(lastUpdatedTimestamp, ((Number) ts).longValue()); | ||
| } |
There was a problem hiding this comment.
Can we extract maximum inference to a method? Looks boiler plate.
|
|
||
| // Handle DELETED_ELEM_TS_FIELD_NAME as a List/Collection | ||
| Object deletedTimestamps = fieldRecord.get(DELETED_ELEM_TS_FIELD_NAME); | ||
| if (deletedTimestamps instanceof List) { |
There was a problem hiding this comment.
Does instanceof List check covers all types of collection in avro?
|
Hi there. This pull request has been inactive for 30 days. To keep our review queue healthy, we plan to close it in 7 days unless there is new activity. If you are still working on this, please push a commit, leave a comment, or convert it to draft to signal intent. Thank you for your time and contributions. |
|
Closing this pull request due to 37 days of inactivity. This is not a judgment on the value of the work. If you would like to continue, please reopen or open a new PR and we will be happy to take another look. Thank you again for contributing. |
2 participants
Problem Statement
Caused by: java.lang.ClassCastException: class org.apache.avro.generic.GenericData$Array cannot be cast to class [J (org.apache.avro.generic.GenericData$Array is in unnamed module of loader 'app'; [J is in module java.base of loader 'bootstrap')
GenericDataArray treats the collection as list opposed to long[] array. Fixing the same.
Solution
Treat DELETED_ELEM_TS_FIELD_NAME and ACTIVE_ELEM_TS_FIELD_NAME as List/Collection instead of long array
How was this PR tested?
Unit tests