dolthub / doltpy

A Python API for Dolt
Apache License 2.0
55 stars 13 forks source link

include primary_key for upsert to avoid ValueError #143

Open jzcruiser opened 3 years ago

jzcruiser commented 3 years ago

When syncing a single column table from dolt to mysql, sqlalchemy will throw a ValueError complaining about an empty update_dict

CREATE TABLE authority ( name varchar(50) NOT NULL, PRIMARY KEY (name) ) ENGINE=InnoDB DEFAULT CHARSET=utf8;

https://github.com/zzzeek/sqlalchemy/blob/857adaaf867df54d4a023cf19f618fdf1d0f60c9/lib/sqlalchemy/dialects/mysql/dml.py#L150

codecov-io commented 3 years ago

Codecov Report

Merging #143 (d1183e8) into master (e7dfa97) will decrease coverage by 13.37%. The diff coverage is 65.90%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #143       +/-   ##
===========================================
- Coverage   55.06%   41.69%   -13.38%     
===========================================
  Files          25       23        -2     
  Lines        1718      957      -761     
===========================================
- Hits          946      399      -547     
+ Misses        772      558      -214     
Impacted Files Coverage Δ
doltpy/sql/__init__.py 100.00% <ø> (ø)
doltpy/sql/sync/__init__.py 0.00% <0.00%> (ø)
doltpy/sql/sync/dolt.py 0.00% <0.00%> (ø)
doltpy/sql/sync/mysql.py 0.00% <0.00%> (ø)
doltpy/sql/sql.py 77.05% <66.66%> (+0.09%) :arrow_up:
doltpy/cli/__init__.py 100.00% <100.00%> (ø)
doltpy/cli/dolt.py 100.00% <100.00%> (+46.16%) :arrow_up:
doltpy/cli/read.py 100.00% <100.00%> (ø)
doltpy/cli/write.py 100.00% <100.00%> (ø)
doltpy/etl/loaders.py 86.89% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 52b5622...d1183e8. Read the comment docs.

jzcruiser commented 3 years ago

Hi @max-hoffman , I don't know how to increase the code coverage yet. Please kindly advise.

max-hoffman commented 3 years ago

The diff coverage would probably look OK if you merged master, a one line change wouldn't have that big of an effect.