Skip to content
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

feat(query): add time_zone param #69

Merged
merged 3 commits into from
Sep 27, 2021
Merged

Conversation

aniaan
Copy link
Contributor

@aniaan aniaan commented Sep 16, 2021

The time_zone parameter is very important for users in non-UTC time zones. We need to add this parameter to adapt to different time zones.

This parameter affects the processing of the time function in the ES-SQL query.

https://www.elastic.co/guide/en/elasticsearch/reference/current/sql-search-api.html

eg:
To demonstrate the role of time_zone, the DATETIME_PARSE function is used, butDATETIME_PARSE can only be used after ES 7.8, so the ES_SUPPORT_DATETIME_PARSE parameter is added to the env

@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2021

Codecov Report

Merging #69 (4cf73f2) into master (ea7a093) will increase coverage by 0.13%.
The diff coverage is 100.00%.

❗ Current head 4cf73f2 differs from pull request most recent head de19337. Consider uploading reports for the commit de19337 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage   94.52%   94.66%   +0.13%     
==========================================
  Files          15       15              
  Lines         932      956      +24     
==========================================
+ Hits          881      905      +24     
  Misses         51       51              
Flag Coverage Δ
python 94.66% <100.00%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
es/elastic/api.py 87.23% <ø> (ø)
es/opendistro/api.py 96.18% <ø> (ø)
es/baseapi.py 90.44% <100.00%> (+0.18%) ⬆️
es/tests/test_dbapi.py 100.00% <100.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 ea7a093...de19337. Read the comment docs.

Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Looks good, can you document this new parameter here: https://github.com/preset-io/elasticsearch-dbapi#connection-parameters

related Superset issue: apache/superset#16726

@aniaan
Copy link
Contributor Author

aniaan commented Sep 27, 2021

Looks good, can you document this new parameter here: https://github.com/preset-io/elasticsearch-dbapi#connection-parameters

related Superset issue: apache/superset#16726

The documentation has been updated, please review

@dpgaspar dpgaspar merged commit 02959e9 into preset-io:master Sep 27, 2021
@aniaan aniaan deleted the feature/param branch September 28, 2021 03:39
@dpgaspar dpgaspar mentioned this pull request Sep 30, 2021
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.

3 participants