PDGGK opened a new pull request, #39610:
URL: https://github.com/apache/shardingsphere/pull/39610

   Changes proposed in this pull request:
   
     - Truncate timestamps to the second with `Math.floorDiv`, so pre-epoch 
values land in the second they belong to.
   
   `isMatched` compares `Timestamp` columns at second granularity, 
deliberately, to avoid precision differences between heterogeneous databases:
   
   ```java
   return ((Timestamp) thisColumnValue).getTime() / 1000L * 1000L
       == ((Timestamp) thatColumnValue).getTime() / 1000L * 1000L;
   ```
   
   Integer division truncates towards zero rather than downwards, so for a 
negative epoch value it rounds *up* — `-1500` becomes `-1000`, putting a 
timestamp in the second above the one it is in. Above the epoch the two agree; 
below it they do not, and the comparison goes wrong in both directions:
   
   | the two values (ms) | apart | current | with `floorDiv` |
   |---|---|---|---|
   | `-1500`, `-1000` | 500 ms, different seconds | **matched** | differ |
   | `-500`, `400` | 900 ms, straddles the epoch | **matched** | differ |
   | `-1`, `1` | 2 ms, different seconds | **matched** | differ |
   | `-1000`, `-999` | 1 ms, same second | **differ** | matched |
   
   The first three are the ones that matter: a data consistency check reports 
two rows as equal while they hold different timestamps, which is the one answer 
the check exists to rule out. The last is the mirror image — two values inside 
the same second reported as a mismatch, so the check fails on data that is in 
fact consistent.
   
   Any table with timestamps before 1970-01-01 is affected — dates of birth, 
historical records, backfilled series.
   
   ### Tests
   
   `assertTimestampEqualsBeforeEpoch` covers all four rows of that table. On 
`master` the first assertion already fails:
   
   ```
   DataConsistencyCheckUtilsTest.assertTimestampEqualsBeforeEpoch:66
   expected: <false> but was: <true>
   ```
   
   The existing `assertTimestampEquals` is untouched and still passes, so the 
above-epoch behaviour and the sub-second tolerance are unchanged.
   
   `mvn test -pl kernel/data-pipeline/core` — 322 tests, all passing.
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following 
options:
   - [x] My code follows the [code of 
conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) 
of this project.
   - [x] I have self-reviewed the commit code.
   - [ ] I have (or in comment I request) added corresponding labels for the 
pull request.
   - [x] I have passed maven check locally : `./mvnw clean install -B -T1C 
-Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
   - [ ] I have made corresponding changes to the documentation.
   - [x] I have added corresponding unit tests for my changes.
   - [ ] I have updated the Release Notes of the current development version. 
For more details, see [Update Release 
Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
   


-- 
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]

Reply via email to