-
Notifications
You must be signed in to change notification settings - Fork 14k
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: fix table existence validation function #11066
Conversation
ddd61a5
to
65cb654
Compare
65cb654
to
76f1e1c
Compare
superset/connectors/sqla/models.py
Outdated
@@ -453,7 +453,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at | |||
owner_class = security_manager.user_model | |||
|
|||
__tablename__ = "tables" | |||
__table_args__ = (UniqueConstraint("database_id", "table_name"),) | |||
__table_args__ = (UniqueConstraint("database_id", "schema", "table_name"),) |
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.
@john-bodley - do you know if there are any issues with changing table_args here?
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.
@bkyryliuk this will not work per #5449.
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.
thanks @john-bodley, reverted this line
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.
This looks like a good enough solution, given the constraints.
* Fix table existance validation function * Drop left over table name index in mysql db * Do not modify model Co-authored-by: bogdan kyryliuk <[email protected]>
SUMMARY
TEST PLAN