Skip to content

Treat empty nullable CSV fields as NULL during sync - #78

Merged
alastairtree merged 2 commits into
mainfrom
copilot/map-empty-string-to-null
Sep 15, 2026
Merged

alastairtree merged 2 commits into
mainfrom
copilot/map-empty-string-to-null

Conversation

Copilot AI commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Empty CSV cells in nullable typed columns were not handled consistently: SQLite stored "", while PostgreSQL rejected numeric empties before they could become NULL. This updates sync behavior so nullable: true is sufficient on its own and lookup: { "": null } is no longer required for ordinary empty-field handling.

  • Behavior change

    • Convert empty CSV cells to NULL for nullable columns after lookup processing
    • Apply this consistently across configured types, including integer, bigint, float, double, boolean, date, datetime, timestamp, text, string, and varchar
  • Lookup interaction

    • Preserve explicit lookup results
    • Only treat values as NULL when the final transformed value is still empty, so non-empty replacements from lookup continue to work as before
  • Regression coverage

    • Add integration coverage for empty nullable values across both SQLite and PostgreSQL
    • Cover all supported configured data types to prevent backend- or type-specific regressions
  • Versioning

    • Bump the package version from 0.7.0 to 0.7.1
columns:
  density:
    db_column: density
    type: float
    nullable: true

With this change, an empty density field in the CSV now syncs as database NULL without requiring:

lookup:
  "": null

Copilot AI and others added 2 commits September 15, 2026 09:41
Co-authored-by: alastairtree <6273429+alastairtree@users.noreply.github.com>
Co-authored-by: alastairtree <6273429+alastairtree@users.noreply.github.com>
Copilot AI changed the title Fix nullable empty CSV values syncing as empty strings Treat empty nullable CSV fields as NULL during sync Sep 15, 2026
Copilot AI requested a review from alastairtree September 15, 2026 09:45
@alastairtree
alastairtree marked this pull request as ready for review September 15, 2026 09:52
@alastairtree
alastairtree enabled auto-merge (squash) September 15, 2026 09:52
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (Linux 3.14)

514 tests  +2   514 ✅ +2   39s ⏱️ +2s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit cadc6a9. ± Comparison against base commit 486a80a.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/crump
  config.py 1047
Project Total  

This report was generated by python-coverage-comment-action

@alastairtree
alastairtree merged commit 105770e into main Sep 15, 2026
12 checks passed
@alastairtree
alastairtree deleted the copilot/map-empty-string-to-null branch September 15, 2026 09:53
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.03%. Comparing base (486a80a) to head (cadc6a9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/crump/config.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   88.00%   88.03%   +0.03%     
==========================================
  Files          17       17              
  Lines        2759     2759              
==========================================
+ Hits         2428     2429       +1     
+ Misses        331      330       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

2 participants