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

Improve and document foreign_keys=... argument to insert/create/etc #17

Closed
simonw opened this issue Feb 24, 2019 · 7 comments
Closed

Improve and document foreign_keys=... argument to insert/create/etc #17

simonw opened this issue Feb 24, 2019 · 7 comments

Comments

@simonw
Copy link
Owner

simonw commented Feb 24, 2019

The foreign_keys= argument to table.insert_all() and friends can be used to specify foreign key relationships that should be created.

It is not yet documented. It also requires you to specify the SQLite type of each column, even though this can be detected by introspecting the referenced table:

cols = [c for c in self.db[other_table].columns if c.name == other_column]
cols[0].type

Relates to #2

@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

It looks like the type information isn't actually used for anything at all, so this:

fresh_db["m2m"].insert(
{"one_id": 1, "two_id": 1},
foreign_keys=(
("one_id", "INTEGER", "one", "id"),
("two_id", "INTEGER", "two", "id"),
),
)

Could actually be written like this:

    fresh_db["m2m"].insert(
        {"one_id": 1, "two_id": 1},
        foreign_keys=(
            ("one_id", "one", "id"),
            ("two_id", "two", "id"),
        ),
)

@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

Sanity checking those foreign keys would be worthwhile.

@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

This involves a breaking API change. I need to call that out in the README and also fix my two other projects which use the old four-tuple version of foreign_keys=:

https://github.com/simonw/db-to-sqlite/blob/c2f8e93bc6bbdfd135de3656ea0f497859ae49ff/db_to_sqlite/cli.py#L30-L42

And

https://github.com/simonw/russian-ira-facebook-ads-datasette/blob/e7106710abdd7bdcae035bedd8bdaba75ae56a12/fetch_and_build_russian_ads.py#L71-L74

I'll also need to set a minimum version for sqlite-utils in the db-to-sqlite setup.py:

https://github.com/simonw/db-to-sqlite/blob/c2f8e93bc6bbdfd135de3656ea0f497859ae49ff/setup.py#L25

simonw added a commit to simonw/db-to-sqlite that referenced this issue Feb 24, 2019
A breaking change is shipping shortly:

simonw/sqlite-utils#17
@simonw simonw closed this as completed in 557dc3f Feb 24, 2019
@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

Re-opening this until I've fixed the other two projects.

@simonw simonw reopened this Feb 24, 2019
@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

Need to put out a new release of sqlite-utils so db-to-sqlite can depend on it.

@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

simonw added a commit to simonw/russian-ira-facebook-ads-datasette that referenced this issue Feb 24, 2019
simonw added a commit to simonw/db-to-sqlite that referenced this issue Feb 24, 2019
@simonw
Copy link
Owner Author

simonw commented Feb 24, 2019

Both projects have been upgraded.

@simonw simonw closed this as completed Feb 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant