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

add support for HH:mm:ss #850

Merged
merged 3 commits into from
Nov 25, 2020
Merged

add support for HH:mm:ss #850

merged 3 commits into from
Nov 25, 2020

Conversation

rupal-bq
Copy link
Contributor

Issue: #849

Description of changes:

  • added STRICT_DATE_FORMATTER

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 Nov 24, 2020 •
edited
Loading

Codecov Report

Merging #850 (583c926) into develop (7f21461) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #850   +/-   ##
==========================================
  Coverage      99.85%   99.85%           
- Complexity      2102     2115   +13     
==========================================
  Files            209      214    +5     
  Lines           4678     4738   +60     
  Branches         308      308           
==========================================
+ Hits            4671     4731   +60     
  Misses             5        5           
  Partials           2        2
Impacted Files Coverage Δ Complexity Δ
...search/data/value/ElasticsearchDateFormatters.java 100.00% <100.00%> (ø) 1.00 <0.00> (ø)
...arch/data/value/ElasticsearchExprValueFactory.java 100.00% <100.00%> (ø) 55.00 <0.00> (ø)
...orelasticsearch/sql/ppl/utils/ArgumentFactory.java 98.38% <0.00%> (-0.17%) 28.00% <0.00%> (-4.00%)
...icsearch/sql/ppl/utils/PPLQueryDataAnonymizer.java 99.00% <0.00%> (-0.01%) 23.00% <0.00%> (ø%)
...endistroforelasticsearch/sql/executor/Explain.java 100.00% <0.00%> (ø) 27.00% <0.00%> (ø%)
...ndistroforelasticsearch/sql/analysis/Analyzer.java 100.00% <0.00%> (ø) 43.00% <0.00%> (ø%)
...troforelasticsearch/sql/ppl/parser/AstBuilder.java 100.00% <0.00%> (ø) 33.00% <0.00%> (ø%)
...orelasticsearch/sql/sql/parser/AstSortBuilder.java 100.00% <0.00%> (ø) 7.00% <0.00%> (ø%)
...elasticsearch/sql/planner/logical/LogicalSort.java 100.00% <0.00%> (ø) 2.00% <0.00%> (ø%)
...asticsearch/sql/planner/physical/SortOperator.java 100.00% <0.00%> (ø) 11.00% <0.00%> (-1.00%)
... and 13 more

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 7f21461...583c926. Read the comment docs.

chloe-zh
chloe-zh approved these changes Nov 24, 2020
View reviewed changes
Copy link
Member

@chloe-zh chloe-zh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

penghuo
penghuo reviewed Nov 24, 2020
View reviewed changes
Copy link
Contributor

@penghuo penghuo left a comment

Choose a reason for hiding this comment

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

Please add UT to cover the STRICT_DATE_FORMATTER

@rupal-bq rupal-bq changed the title add datetime format YYYY-MM-DD HH:mm:ss add datetime format HH:mm:ss Nov 25, 2020
@rupal-bq rupal-bq changed the title add datetime format HH:mm:ss add support for HH:mm:ss Nov 25, 2020
@rupal-bq
Copy link
Contributor Author

@penghuo @dai-chen this change fixed the issue with calcs data but I was wondering if I should create constructTime similar to constructTimestamp instead ?

dai-chen
dai-chen approved these changes Nov 25, 2020
View reviewed changes
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!

@penghuo penghuo merged commit 200a76d into opendistro-for-elasticsearch:develop Nov 25, 2020
penghuo pushed a commit that referenced this pull request Dec 15, 2020
@rupal-bq @penghuo
add support for HH:mm:ss (#850)
aa025a7 
* add STRICT_DATE_FORMATTER

* fix chekstyle error

* update for HH:MM:SS

Co-authored-by: Rupal Mahajan <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@penghuo penghuo penghuo approved these changes

@chloe-zh chloe-zh chloe-zh approved these changes

@dai-chen dai-chen dai-chen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rupal-bq @penghuo @chloe-zh @dai-chen