[AMORO-3632]: data-retention, add support for partition column type Date #3665
Conversation
OK, no problem |
@zhoujinsong I have added a new test case. Can you review the code to see if there is any problem? tks! |
| @@ -764,6 +764,9 @@ private Comparable<?> getExpireValue( | |||
| .format( | |||
There was a problem hiding this comment.
Could you please help to add some comments for this function, thanks
| @@ -1037,8 +1047,13 @@ private boolean canBeExpireByPartitionValue( | |||
| } | |||
|
|
|||
| int compared = filePartitionValue.compareTo(partitionUpperBound); | |||
| Boolean compareResult = | |||
| expireField.type() == Types.StringType.get() ? compared <= 0 : compared < 0; | |||
| Boolean compareResult; | |||
There was a problem hiding this comment.
why do we add DateType to the special case? if the bound is the problem, could we calculate the bound with `(int) ((expireTimestamp + 24 * 60 * 60 * 1000) + / (24 * 60 * 60 * 1000))
There was a problem hiding this comment.
I am re-adjusting the date boundary value calculation problem. This part still maintains the original string type calculation logic.
| @@ -373,6 +416,10 @@ private void testUnKeyedFileLevel() { | |||
| expected = | |||
| Lists.newArrayList( | |||
| createRecord(2, "222", parseMillis("2022-01-03T12:00:00"), "2022-01-03T12:00:00")); | |||
| } else if (expireByDate()) { | |||
There was a problem hiding this comment.
Could you please add some more tests to cover the bound case(eg, 2022-01-03T23:59:59, 2022-01-03T00:00:00, 2022-01-03T00:00:01)
|
@klion26 @zhoujinsong teachers，Could you please help me look at the code again? I have added a few boundary test cases, but there are still some problems during the test. I am a little confused. test command： mvn test -Dtest=TestDataExpire#testDateTypeBoundaryConditions* -pl amoro-ams |
Why are the changes needed?
Close #3632
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation