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

find_column_type fix for snowflake dates #226

Merged
merged 2 commits into from
Sep 6, 2023

Conversation

rhaffar
Copy link
Contributor

@rhaffar rhaffar commented Aug 29, 2023

  • Updated find_column_type method to only map columns types to date if they contain data with valid Snowflake date formats
  • Added warehouse parameter to find_column_type method to differentiate column type mappings based on warehouse type (only affects dates)
  • Updated test and internal calls of find_column_type

Fixes #221

@CLAassistant
Copy link

CLAassistant commented Aug 29, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@fdosani fdosani left a comment

Choose a reason for hiding this comment

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

LGTM. Was thinking through the logic since it has been a while since I went through this stuff. Overall it looks good. thanks for updating the unit tests and adding in extras there.

@fdosani fdosani changed the title locopy fix find_column_type fix for snowflake dates Sep 6, 2023
@rhaffar
Copy link
Contributor Author

rhaffar commented Sep 6, 2023

LGTM. Was thinking through the logic since it has been a while since I went through this stuff. Overall it looks good. thanks for updating the unit tests and adding in extras there.

Thanks for reviewing Faisal! Any idea how long the Whitesource checks tend to take? Can't merge until they are done.

@fdosani fdosani merged commit 9575f1b into capitalone:develop Sep 6, 2023
@fdosani fdosani mentioned this pull request Sep 6, 2023
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.

find_column_type incorrectly identifying certain date related columns
3 participants