-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Handle Invalid timestamps #9355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9355 +/- ##
============================================
+ Coverage 63.38% 69.71% +6.33%
+ Complexity 4755 4710 -45
============================================
Files 1831 1890 +59
Lines 97999 100760 +2761
Branches 14990 15352 +362
============================================
+ Hits 62115 70246 +8131
+ Misses 31299 25523 -5776
- Partials 4585 4991 +406
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
58e966f to
f80344c
Compare
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFieldSpec.java
Outdated
Show resolved
Hide resolved
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFieldSpec.java
Outdated
Show resolved
Hide resolved
...in/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java
Outdated
Show resolved
Hide resolved
f80344c to
a79884e
Compare
d56bd57 to
7774ed5
Compare
Jackie-Jiang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
| long sampleTimestampValue = (sampleValue instanceof String) ? formatSpec.fromFormatToMillis((String) sampleValue) | ||
| : formatSpec.fromFormatToMillis(String.valueOf(sampleValue)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| long sampleTimestampValue = (sampleValue instanceof String) ? formatSpec.fromFormatToMillis((String) sampleValue) | |
| : formatSpec.fromFormatToMillis(String.valueOf(sampleValue)); | |
| long sampleTimestampValue; | |
| try { | |
| sampleTimestampValue = formatSpec.fromFormatToMillis(sampleValue.toString()); | |
| } catch (Exception e) { | |
| throw new IllegalArgumentException(...); | |
| } |
|
Let's also update the user doc about the new field |
* Add sample value in datetime fieldspec to check correct format * Bug fix and add test cases for sample value * Fix linting * throw exception in case of error Co-authored-by: Kartik Khare <[email protected]>
sampleValuefield to check the correct timestamp format during schema creation.