Skip to content
This repository was archived by the owner on Aug 2, 2022. It is now read-only.

Allow Timestamp/Datetime values to use up to 6 digits of microsecond precision #988

Conversation

jordanw-bq
Copy link
Contributor

Issue #, if available: #986

Description of changes:
Add formatter to allow for variable amount of microsecond precision (up to 6 digits)

  • add additional tests around microsecond precision
  • update TimeValue to align with Timestamp/Datetime changes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #988 (5990495) into develop (9a08770) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #988   +/-   ##
==========================================
  Coverage      99.87%   99.87%           
- Complexity      2381     2397   +16     
==========================================
  Files            234      234           
  Lines           5477     5513   +36     
  Branches         357      357           
==========================================
+ Hits            5470     5506   +36     
  Misses             5        5           
  Partials           2        2           
Impacted Files Coverage Δ Complexity Δ
...lasticsearch/sql/data/model/ExprDatetimeValue.java 100.00% <100.00%> (ø) 13.00 <1.00> (ø)
...forelasticsearch/sql/data/model/ExprTimeValue.java 100.00% <100.00%> (ø) 9.00 <1.00> (+1.00)
...asticsearch/sql/data/model/ExprTimestampValue.java 100.00% <100.00%> (ø) 13.00 <0.00> (ø)
...opendistroforelasticsearch/sql/expression/DSL.java 100.00% <0.00%> (ø) 132.00% <0.00%> (+3.00%)
...ticsearch/sql/ppl/parser/AstExpressionBuilder.java 100.00% <0.00%> (ø) 28.00% <0.00%> (+2.00%)
...h/sql/expression/function/BuiltinFunctionName.java 100.00% <0.00%> (ø) 3.00% <0.00%> (ø%)
...ion/operator/predicate/UnaryPredicateOperator.java 100.00% <0.00%> (ø) 23.00% <0.00%> (+10.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a08770...5990495. Read the comment docs.

@chloe-zh
Copy link
Member

Could you add integration test in sql/DateTimeFunctionIT.java for SQL (if comparison test is not applicable) and ppl/DateTimeFunctionIT.java for PPL in integ-test module? The rest LGMT, thanks!

Copy link
Member

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@dai-chen dai-chen merged commit 47d50d2 into opendistro-for-elasticsearch:develop Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants