-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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: datatype tracking issue on virtual dataset #20088
fix: datatype tracking issue on virtual dataset #20088
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20088 +/- ##
===========================================
- Coverage 66.47% 54.55% -11.92%
===========================================
Files 1727 1727
Lines 64724 64732 +8
Branches 6822 6822
===========================================
- Hits 43024 35314 -7710
- Misses 19969 27687 +7718
Partials 1731 1731
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Looks good! :)
Looks like there's a test failing. |
@@ -21,6 +21,7 @@ | |||
from typing import Any, Dict, List, Optional, Pattern, Tuple, TYPE_CHECKING | |||
|
|||
from flask_babel import gettext as __ | |||
from psycopg2.extensions import binary_types, string_types |
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.
I think this import should be moved into get_datatype
, as psycopg2
isn't a required dependency
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.
Fixed by #20543
* Fix datatype tracking issue on virtual dataset * fix pytest issue on postgresql
SUMMARY
Column types not detected when creating Virtual Dataset.
If we create a virtual dataset, and try to create a chart based on it, some column types may not be detected as it was defined in original dataset. It was because some columns had no any non-null values when we run the SQL to create a dataset.
The previous version was tracking column type of columns from the query result values, and that's why some columns had null type even though it had exact type in original dataset.
To fix this, we need to track the column type from original dataset, not a SQL query running result value.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE
![image](https://user-images.githubusercontent.com/39701522/168673931-73577f6d-da20-4179-b797-1468d9441466.png)
AFTER:
![Screen Shot 2022-05-16 at 16 01 45](https://user-images.githubusercontent.com/39701522/168673789-edcd62f3-9fd2-4d72-a54d-789f9bdb139a.png)
![Screen Shot 2022-05-16 at 16 03 10](https://user-images.githubusercontent.com/39701522/168673819-13c6ecca-6370-407a-9d81-fcb9027f7072.png)
TESTING INSTRUCTIONS
Dataset for virtual dataset.
sqllab_query_publicconnectmiles_all_reservation_stats_20220412T210849.csv
ADDITIONAL INFORMATION