-
Notifications
You must be signed in to change notification settings - Fork 2
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(spanddl): support dropping interleaved tables #501
Conversation
thall
commented
Jan 3, 2024
•
edited
Loading
edited
- test(spanddl): possibility to specify expected error for ddl statement
- feat: throw error if dropping table with interleaved tables
- fix(spanddl): support dropping interleaved tables
If an error occurred, an assertion on database schema is not needed.
`DROP TABLE Albums`, | ||
}, | ||
errorDdlIndex: 1, | ||
errorContains: "DROP TABLE: table Albums does not exist", |
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.
errorContains
was unused before this pr.
assert.ErrorContains(t, err, tt.errorContains) | ||
break | ||
return |
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.
exit early if any ddl statement thrown an error
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #501 +/- ##
==========================================
- Coverage 44.66% 43.45% -1.22%
==========================================
Files 21 20 -1
Lines 4267 4582 +315
==========================================
+ Hits 1906 1991 +85
- Misses 2249 2483 +234
+ Partials 112 108 -4
|
Before the parent would have an reference to the dropped table, which caused an type error since the type could not be resolved. This PR removes the child reference in the parent table.
27e15aa
to
33c5c38
Compare
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.
LGTM! 🌟