update datetime tests to stay within ae bounds#5534
Conversation
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
RyanL1997
left a comment
There was a problem hiding this comment.
Thanks for the change @Swiddis.
one small nit on the description — the i64 cap that kicks in at year 2262 is from nanosecond precision, not microsecond (μs would actually let us go way past year 290k). doesn't change anything about the fix itself since 2242 is under 2262 either way.
| public void init() throws Exception { | ||
| super.init(); | ||
| loadIndex(Index.DATA_TYPE_NONNUMERIC); | ||
| loadIndex(Index.DATETIME_SIMPLE); |
There was a problem hiding this comment.
qq: does the sql variant's testCompare actually need this index loaded? scrolling down to the method body, it is just select <expr> AS <name> with no FROM clause, so i don't think this load is reaching any query.
There was a problem hiding this comment.
lol, probably. i think it's just copied from ppl maybe since ppl doesn't support bare queries without indices. won't rock the boat with it though
Description
Simplifies the test indices & changes some values, preserves all the same core semantics of the test. Parquet/Datafusion use 64-bit microsecond epochs which cap at year 2262.
Related Issues
N/A
Check List
--signoffor-s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.