-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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(ingest/snowflake): Fixing table rename query handling #12852
fix(ingest/snowflake): Fixing table rename query handling #12852
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
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.
Seems pretty reasonable - would be good to add some unit tests for the parse_ddl_query
(basically just with a given object_modified_by_ddl value, make sure it generates the right operation)
query=query, | ||
session_id=session_id, | ||
timestamp=timestamp, | ||
), "Processing ALTER ... RENAME should result in a proper TableRename object" |
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.
in general, we don't need comments on asserts - the test feels pretty clear already
For table renames
operationType
inobject_modified_by_ddl
dictionary isALTER
. We need to checkquery_type
to see whether it was table-rename-kind-of-alter. Taking this into consideration I added this value toparse_ddl_query
. Considering pre-existing logic there I had to swap order ofif
conditions to achieve intended behavior (the logic didn't change beyond that).