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.

Sign up for GitHub

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

Jump to bottom

update tables unique key #15833

Closed
wants to merge 1 commit into from
Closed

update tables unique key #15833

wants to merge 1 commit into from

Conversation

FishermanZzhang
Copy link

@FishermanZzhang FishermanZzhang commented Jul 22, 2021 •
edited
Loading

SUMMARY

update dataset(tables) unique key

i just test on mysql. and I know the sqlite no support.

i found this code
#9882

but, not worked. why?

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@FishermanZzhang FishermanZzhang requested a review from a team as a code owner July 22, 2021 08:43
@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@john-bodley
Copy link
Member

john-bodley commented Jul 23, 2021 •
edited
Loading

@FishermanZzhang, sadly this wont work as every database encodes uniqueness with NULL differently.

@FishermanZzhang
Copy link
Author

@FishermanZzhang, sadly this wont work as every database encodes uniqueness with NULL differently.

In my opinion. SQLALCHEMY_DATABASE_URI = 'mysql://{user}:{passwd}@{host}/superset is best.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2021

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

13 similar comments
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2021

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2021

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2021

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2021

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

craig-rueda
craig-rueda requested changes Oct 25, 2021
View reviewed changes
superset/migrations/versions/644c950f63d7_upgrade_unique_constrain.py
def upgrade():
try:
op.create_unique_constraint('tables_unique', 'tables', ['table_name', 'schema', 'database_id'])
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

This should catch the specific sqlite error

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

2 similar comments
@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @FishermanZzhang Your base branch master has just also updated superset/migrations.

❗ Please consider rebasing your branch to avoid db migration conflicts.

betodealmeida
betodealmeida requested changes Nov 24, 2021
View reviewed changes
Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

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

Can you give context on why we'd want to add this constraint?

Also, this model is going away soon, see SIP-68 (#14909). The new model separates tables (which will have a uniqueness constraint) from datasets (which can point to the same table).

@betodealmeida
Copy link
Member

Closing this, since #14909 will replace it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@betodealmeida betodealmeida betodealmeida requested changes

@craig-rueda craig-rueda craig-rueda requested changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@FishermanZzhang @john-bodley @betodealmeida @craig-rueda