adenes commented on pull request #4677: URL: https://github.com/apache/nifi/pull/4677#issuecomment-731271678
Thanks @exceptionfactory for the review. Actually your recommended change is the revert of the original commit that caused these test to fail (see: https://github.com/apache/nifi/commit/83948db989bf2f682dfa18c56293ec431f546e1c#diff-98d8c798133bf7b6d8a2b49a35139863b6ac85c6001787a62b3c6117abafb440L669) I think we have a more generic issue, using `java.sql.Date` for representing dates is not the best solution. Being a subclass of `java.util.Data` means that under the hood it stores the time too, just doesn't expose it. Storing the time leads to a "date skew" issue: when 2020-11-20 is represented as 2020-11-20 00:00 UTC but if later it's converted to a different timezone (e.g. EST, UTC-5) the same instant is presented as 2020-11-19 19:00 EST, thus the date part has changed. This case is covered by a newly added test case: https://github.com/apache/nifi/blob/main/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestConvertRecord.java#L378 My opinion is that in case of dates time zone doesn't make sense (i.e. 2020-11-19 is the same date in every time zone, even if in some it starts earlier), that's why I removed the `toLocalDate()` call in the previously mentioned commit to fix this issue. Unfortunately this led to these two failing tests, but I think the tests are incorrect. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
