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

Cannot create table with same name from a different datasource #3829

Closed
3 tasks
frankfarrell opened this issue Nov 10, 2017 · 27 comments
Closed
3 tasks

Cannot create table with same name from a different datasource #3829

frankfarrell opened this issue Nov 10, 2017 · 27 comments
Labels
need:followup Requires followup

Comments

@frankfarrell
Copy link

Make sure these boxes are checked before submitting your issue - thank you!

  • I have checked the superset logs for python stacktraces and included it here as text if any
  • I have reproduced the issue with at least the latest released version of superset
  • I have checked the issue tracker for the same issue and I haven't found one similar

Superset version

0.19.1

Expected results

That you could add the same table from a differnet datasource

Actual results

(raised as a result of Query-invoked autoflush; consider using a session.no_autoflush block if this flush is occurring prematurely) (sqlite3.IntegrityError) UNIQUE constraint failed: tables.table_name [SQL: 'INSERT INTO tables (created_on, changed_on, description, default_endpoint, is_featured, filter_select_enabled, "offset", cache_timeout, params, perm, table_name, main_dttm_col, database_id, fetch_values_predicate, user_id, schema, sql, created_by_fk, changed_by_fk) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'] [parameters: ('2017-11-10 16:36:45.635385', '2017-11-10 16:36:45.635417', None, None, 0, 0, 0, None, None, None, '<table_name>', None, 2, None, None, '', None, None, None)]

Steps to reproduce

We have a few datasources using the same schema. We would like to configure query tables from each.

It seems an easy fix would be just to change this line: https://github.com/apache/incubator-
superset/blob/master/superset/migrations/versions/4e6a06bad7a8_init.py#L84
to sa.UniqueConstraint('table_name', 'database_id'). This line does have such a constraint ( with schema too), https://github.com/apache/incubator-superset/blob/1ea4521d0c116d96b51749613347279669529dd0/superset/connectors/sqla/models.py#L188

Its similar to https://github.com/apache/incubator-superset/pull/3583/files, so perhaps a fix could be included in that PR too?

Thanks, great project by the way!

@xrmx
Copy link
Contributor

xrmx commented Nov 10, 2017

Feel free to open a PR

@frankfarrell
Copy link
Author

Will do

@mistercrunch
Copy link
Member

I think this open PR solves this: #3583

@frankfarrell
Copy link
Author

frankfarrell commented Nov 15, 2017

superset-table-bug
This is the error. Its very similar to #3583 but not the same.
For slices it may be necessary to show <table_name>/ rather than just table name?

@frankfarrell
Copy link
Author

https://github.com/apache/incubator-superset/compare/master...frankfarrell:tables-constraints?expand=1
It just needs the alembic upgrade/downgrade functiosn to match now

@frankfarrell
Copy link
Author

Root cause here is that the alembic migrations scripts first creates a unique constraint on table name:
https://github.com/apache/incubator-superset/blob/master/superset/migrations/versions/4e6a06bad7a8_init.py#L84
Later revisions try to create the correct unique constraint on table)name, schema and database:
https://github.com/apache/incubator-superset/blob/master/superset/migrations/versions/b4456560d4f3_change_table_unique_constraint.py#L21
and
https://github.com/apache/incubator-superset/blob/master/superset/migrations/versions/3b626e2a6783_sync_db_with_models.py#L65

However, sqlite doesn't allow dropping unnamed unique constraints:
https://stackoverflow.com/questions/42013265/remove-unique-constraint-on-a-column-in-sqlite-database

Solution would be to rename table, create new table and copy over data in a new migration. Or use some other db :)

@mistercrunch
Copy link
Member

We could also alter the creation of the constraint in the first place. Won't help existing installations but it will help future ones.

@frankfarrell
Copy link
Author

frankfarrell commented Nov 20, 2017

@mistercrunch That's also an option, I wasn't sure if you wanted to keep the migration scripts immutable or not. Chances are that nobody will upgrade an sqlite db in place.
In any case I have the correct migration done in the PR: #3885, which fixes the issue

@mistercrunch
Copy link
Member

In general we want to keep the migrations static but in cases like this I think it's ok to alter them, as long as downstream migration account for both branches in history.

I'll let @john-bodley review the PR since he's done something very similar on the Druid side recently.

@john-bodley
Copy link
Member

@frankfarrell are you certain you weren't able to delete an unnamed uniqueness constraint for SQLite? There's a section on the Alembic documentation about dealing with constraints in SQLite.

I was able to leverage this logic for dropping uniqueness constraints here

@jerowe
Copy link
Contributor

jerowe commented Jan 5, 2018

I'm having the same issue. Is this still being worked on?

@frankfarrell
Copy link
Author

I created this PR #3885 which solves the issue. Its uses plain SQL, I haven't updated it to use alembic as described by @john-bodley above. Should I do that or does it look ok to merge as is?

@asyd
Copy link

asyd commented Apr 4, 2018

Any news by chance ?

@asyd
Copy link

asyd commented Apr 4, 2018

Oh, just tested 0.24.0 and seem it's working, so maybe someone should close this issue?

@SanjayJosh
Copy link

SanjayJosh commented Jul 2, 2018

I am using 0.25.6 . This still seems to be an issue.
screen shot 2018-07-02 at 2 35 46 pm
I tried adding another nyc_data_external from a different database customer_data, which also happens to have nyc_data_external table. The error was still thrown, @mistercrunch

@SanjayJosh
Copy link

SanjayJosh commented Jul 2, 2018

I was thinking, since users generally don't migrate once the app is initialised, how about we add the script to re configure the tables table for sqlite3 as a part of the code when superset init runs.
It could reside well as utils function.

@Preston4tw
Copy link

@SanjayJosh it might be worth opening a new issue, this may be a regression.

@raof01
Copy link

raof01 commented May 19, 2019

I am using 0.25.6 . This still seems to be an issue.
screen shot 2018-07-02 at 2 35 46 pm
I tried adding another nyc_data_external from a different database customer_data, which also happens to have nyc_data_external table. The error was still thrown, @mistercrunch

@SanjayJosh It seems there's no new issue about the regression? Or did you just found it's gone?

@FredericLatour
Copy link

I just can'( believe it's not possible to have two databases with tables having the same name??!!
I must be missing something ... am I ?

@natbusa
Copy link

natbusa commented Sep 10, 2020

This problem might be sqlite specific because of the very peculiar way of working with alembic.
Switch to postgres for metadata will most likely solve the problem.

@andreas-eberle
Copy link
Contributor

@mistercrunch: Can this be reopend? It still isn't fixed. I also experience this issue.
@natbusa: Have you been able to test your suggestion?

@natbusa
Copy link

natbusa commented Sep 25, 2020 via email

@snower
Copy link

snower commented Oct 16, 2020

I found that this problem still exists when using mysql, because the "table_name" unique index created in the "tables" table only contains "table_name" but not "database_id", and after adding "database_id" to the unique index, the table can be created normally.

@rob05c
Copy link
Member

rob05c commented Oct 22, 2020

I just hit the same issue, with the default sqlite metadata source. Fixed it in sqlite3 with

$ sqlite3 ~/.superset/superset.db
CREATE TABLE "tables2" (
⋮ 
        UNIQUE (table_name, database_id),
⋮ 
;
insert into tables2 select * from tables;
drop table tables;
alter table tables2 rename to tables;

Seems like it isn't fixed in the latest stable version, which just needs UNIQUE (table_name), changed to UNIQUE (table_name, database_id),.

$ superset version
Superset 0.37.2

@anthonybu
Copy link

Oh, just tested 0.24.0 and seem it's working, so maybe someone should close this issue?

Still seeing the same issue in 1.0.0

@junlincc junlincc added the need:followup Requires followup label Feb 11, 2021
@FishermanZzhang
Copy link

    __table_args__ = (UniqueConstraint("database_id", "table_name"),)

but,the unique only table_name , not dataset_id

sqlite> .schema tables
CREATE TABLE IF NOT EXISTS "tables" (
        ...
        UNIQUE (table_name),
        FOREIGN KEY(created_by_fk) REFERENCES ab_user (id)
);

as the code say, it show be UNIQUE (database_id, table_name)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need:followup Requires followup
Projects
None yet
Development

No branches or pull requests