Conversation
| EARLIEST("user", 128) AS "first_user", | ||
| LATEST("user", 128) AS "last_user" | ||
| EARLIEST_BY("user", CASE WHEN CAST("timestamp" AS BIGINT) > 0 THEN MILLIS_TO_TIMESTAMP(CAST("timestamp" AS BIGINT)) ELSE TIME_PARSE("timestamp") END, 128) AS "first_user", | ||
| LATEST_BY("user", CASE WHEN CAST("timestamp" AS BIGINT) > 0 THEN MILLIS_TO_TIMESTAMP(CAST("timestamp" AS BIGINT)) ELSE TIME_PARSE("timestamp") END, 128) AS "last_user" |
There was a problem hiding this comment.
This is not a "fix", it is a workaround that allows the test to pass through the underlying code is broken. A better approach is to simply disable the test with a comment.
That is, this "fix" substitutes another test case (which works) for the test case which fails because it is revealing a bug.
There was a problem hiding this comment.
I agree, this isn't a fix, it's a workaround, but the tests that are failing are not to check for aggregations. This is used for a few tests like BatchIngest test and ReindexTest, and any aggregation would work for these tests. Disabling them might make a part of the code untested instead. Should I disable these tests for now?
There was a problem hiding this comment.
How about this. This test did, in fact, find a bug, so it is useful. But, the point of the test is actually to exercise something else.
So, maybe comment out the lines that fail. Ensure we have an Apache issue for the problem. Have that problem point to the test and say that, to see the bug, just uncomment the lines.
Then, go ahead and add your new lines so that the test passes.
This will achieve the short-term goal (clean test) while also ensuring we don't forget to fix the underlying issue.
There was a problem hiding this comment.
Thanks a lot for the suggestion! That sounds like a good way to handle this. I have made the changes.
paul-rogers
left a comment
paul-rogers
left a comment
There was a problem hiding this comment.
Thanks for the fix. LGTM.
3 participants
Due to LATEST when used in combination with an EXTERN source, all values of __time are seen as 0. This leads to some potentially undeterminsitic values being returned from queries.
This fixes flaky failures for the MSQ IT tests and unblocks the CICD.
This PR has: