Hive: Quick fix for broken TestHiveIcebergFilterFactory.testTimestampType test

[pvary]

#2254 modified how the Timestamp type is handled in HiveIcebergFilterFactory.microsFromTimestamp but forgot to update the tests for filter conversion.

The original change (#2254) fixed the Timestamp / Date filters to work not only in UTC timezone but other timezones as well. The change was verified by adding new tests which were testing with multiple timezones, but the original TestHiveIcebergFilterFactory.testTimestampType test was checking the base behavior which were originally wrong and fixed in #2254, so we need to change the test too.

[pvary]

@marton-bod: Thanks for the review!

@shardulm94: Based on the dev list discussion created a quick fix and pushed the change, but I would like to have your review if possible

Thanks,
Peter

[edgarRd]

On these tests that check timezone sensitive types, and with the new changes in #2278 should we check with a few timezones to validate something like in https://github.com/apache/iceberg/blob/master/orc/src/test/java/org/apache/iceberg/orc/TestExpressionToSearchArgument.java#L120 - Thanks!

[pvary]

On these tests that check timezone sensitive types, and with the new changes in #2278 should we check with a few timezones to validate something like in https://github.com/apache/iceberg/blob/master/orc/src/test/java/org/apache/iceberg/orc/TestExpressionToSearchArgument.java#L120 - Thanks!

I am not sure I understand your suggestion correctly, so please correct me if I missed something.

• We have end-to-end tests in TestHiveIcebergStorageHandlerTimezone. There we test the date/timestamp/timestamp withzone - same as in TestExpressionToSearchArgument. Might be useful for add tests with all of the FileFormat-s, but my feeling is that it should be tested on the individual FileFormats, and not here.
• We test the individual conversion in the TestHiveIcebergFilterFactory. I am not fan of this test as it is basically inverting the microsFromTimestamp(Timestamp timestamp) method, and only testing if timestamp.toLocalDateTime() is working correctly or not 😄

[edgarRd]

I agree that this test is not particularly interesting. But, moving forward my suggestion goes to suggest how can we avoid test cases to not be detected by CI when ran in different timezones - as was the case with the changes that lead to this test failing.
If you think we're covered with the existing test harness then that should be enough 👍🏽 .