Skip to content

Test for numeric timestamp parser with string timestamp#9888

Closed
suneet-s wants to merge 1 commit intoapache:masterfrom
suneet-s:test-timestamp-parser
Closed

Test for numeric timestamp parser with string timestamp#9888
suneet-s wants to merge 1 commit intoapache:masterfrom
suneet-s:test-timestamp-parser

Conversation

@suneet-s
Copy link
Contributor

No description provided.

@suneet-s
Copy link
Contributor Author

@samarthjain - I added a test case for #9877 - I think this is the intended behavior. Can you confirm?

Assert.assertEquals("Timestamp of format millis not parsed correctly",
Assert.assertEquals("Timestamp of format millis as long not parsed correctly",
expectedDt, parser.apply(millis));
Assert.assertEquals("Timestamp of format millis as string not parsed correctly",
Copy link
Contributor

@samarthjain samarthjain May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suneet-s - this test case passes fine without the fix in #9877. #9877 was meant for use cases where the ingested column storing time is a numeric column (int, long) that stores time in a format that isn't auto || millis || posix || micro || nano || ruby. If the column is of type string, then the timestamp parser parses the time correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samarthjain makes sense. I initially thought this was a missing test case before your pr. Looking more closely at the tests, I see this is covered by TimestampParserTest#testNano

Closing this PR.

@suneet-s suneet-s closed this May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants