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

db[table].create(..., transform=True) and create-table --transform #468

Merged
merged 13 commits into from Aug 27, 2022

Conversation

simonw
Copy link
Owner

@simonw simonw commented Aug 23, 2022

Work in progress. Still needs documentation and tests (and to cover more cases of things that might have changed).

Refs:


📚 Documentation preview 📚: https://sqlite-utils--468.org.readthedocs.build/en/468/

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #468 (fc38480) into main (c5f8a2e) will decrease coverage by 0.12%.
The diff coverage is 88.88%.

❗ Current head fc38480 differs from pull request most recent head 2f6a64f. Consider uploading reports for the commit 2f6a64f to get more accurate results

@@            Coverage Diff             @@
##             main     #468      +/-   ##
==========================================
- Coverage   96.60%   96.47%   -0.13%     
==========================================
  Files           6        6              
  Lines        2590     2640      +50     
==========================================
+ Hits         2502     2547      +45     
- Misses         88       93       +5     
Impacted Files Coverage Δ
sqlite_utils/cli.py 95.86% <66.66%> (-0.09%) ⬇️
sqlite_utils/db.py 97.31% <90.19%> (-0.23%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@simonw simonw added this to the 3.29 milestone Aug 27, 2022
@simonw
Copy link
Owner Author

simonw commented Aug 27, 2022

The main challenge here is coming up with comprehensive tests. The cases I need to cover are from this block of code:

def create(
self,
columns: Dict[str, Any],
pk: Optional[Any] = None,
foreign_keys: Optional[ForeignKeysType] = None,
column_order: Optional[List[str]] = None,
not_null: Iterable[str] = None,
defaults: Optional[Dict[str, Any]] = None,
hash_id: Optional[str] = None,
hash_id_columns: Optional[Iterable[str]] = None,
extracts: Optional[Union[Dict[str, str], List[str]]] = None,
if_not_exists: bool = False,
) -> "Table":

@simonw
Copy link
Owner Author

simonw commented Aug 27, 2022

For the moment I'm not going to pay attention to foreign_keys changes - I will note that these are not modified in the documentation.

@simonw
Copy link
Owner Author

simonw commented Aug 27, 2022

Interesting challenge with default_value: I need to be able to tell if the default values passed to .create() differ from those in the database already.

Introspecting that is a bit tricky:

>>> import sqlite_utils
>>> db = sqlite_utils.Database(memory=True)
>>> db["blah"].create({"id": int, "name": str}, not_null=("name",), defaults={"name": "bob"})
<Table blah (id, name)>
>>> db["blah"].columns
[Column(cid=0, name='id', type='INTEGER', notnull=0, default_value=None, is_pk=0), Column(cid=1, name='name', type='TEXT', notnull=1, default_value="'bob'", is_pk=0)]

Note how a default value of the Python string bob is represented in the results of PRAGMA table_info() as default_value="'bob'" - it's got single quotes added to it!

So comparing default values from introspecting the database needs me to first parse that syntax. This may require a new table introspection method.

simonw added a commit that referenced this pull request Aug 27, 2022
@simonw
Copy link
Owner Author

simonw commented Aug 27, 2022

@simonw simonw marked this pull request as ready for review August 27, 2022 23:04
@simonw simonw changed the title db[table].create(..., transform=True) - refs #467 db[table].create(..., transform=True) Aug 27, 2022
@simonw simonw changed the title db[table].create(..., transform=True) db[table].create(..., transform=True) and create-table --transform Aug 27, 2022
@simonw simonw merged commit 104f37f into main Aug 27, 2022
simonw added a commit that referenced this pull request Aug 27, 2022
simonw added a commit that referenced this pull request Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant