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

fix: elasticsearch column type mapping #27148

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mapledan
Copy link
Contributor

@mapledan mapledan commented Feb 17, 2024

The datetime field in the CursorDescriptionRow returned by elasticsearch-dbapi is identified as Type.DATETIME=4.
During the creation of the _type_dict, the db_engine_spec.get_datatype method returns an incorrect type.
image

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Creating a dataset using Elasticsearch.
  2. Use the following SQL: select ins_date,cast(ins_date as datetime) as cast_ins_date from my_es.
  3. The column type will be correct type.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Copy link

codecov bot commented Feb 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.48%. Comparing base (1776405) to head (d7e6d62).
Report is 1725 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #27148      +/-   ##
==========================================
+ Coverage   67.14%   69.48%   +2.33%     
==========================================
  Files        1901     1901              
  Lines       74444    74450       +6     
  Branches     8304     8304              
==========================================
+ Hits        49989    51728    +1739     
+ Misses      22403    20670    -1733     
  Partials     2052     2052              
Flag Coverage Δ
hive 53.80% <85.71%> (?)
mysql 78.01% <100.00%> (+<0.01%) ⬆️
postgres 78.11% <100.00%> (+<0.01%) ⬆️
presto 53.75% <85.71%> (?)
python 83.08% <100.00%> (+4.83%) ⬆️
sqlite 77.62% <100.00%> (+<0.01%) ⬆️
unit 56.43% <85.71%> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@john-bodley john-bodley requested a review from villebro February 20, 2024 17:49
@mapledan
Copy link
Contributor Author

@villebro Is there any need for improvement?

@rusackas rusackas marked this pull request as draft January 3, 2025 18:34
@rusackas
Copy link
Member

rusackas commented Jan 3, 2025

Convering this to draft since it has conflicts, and hasn't had the conversation or review pushed forward in 9 months. It may be closed in time, but perhaps we can merge it if the conflict is resolved and conversation rekindled :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants