palewire / django-postgres-copy

Quickly import and export delimited data with Django support for PostgreSQL's COPY command
https://palewi.re/docs/django-postgres-copy/
MIT License
178 stars 48 forks source link

Upsert #156

Open geoffrey-eisenbarth opened 1 year ago

geoffrey-eisenbarth commented 1 year ago

I'm willing to write tests and update documentation if this approach seems sufficient.

Having some trouble getting the test suite to run (getting django.db.utils.OperationalError: fe_sendauth: no password supplied when I run pipenv run python setup.py test); do I need to create a (postgres) database called test?

palewire commented 1 year ago

Thanks for taking a run at it. Tests and documentation will def. be something we want. Yes I think you'll need a database to test against.

Here is how the test suite configures the database https://github.com/palewire/django-postgres-copy/blob/main/.github/workflows/continuous-deployment.yaml#L11-L21

geoffrey-eisenbarth commented 1 year ago

@palewire Thanks for giving the go-ahead. I've got some new tests in progress, but will wait to commit them until I can run them locally.

EDIT: Got testing up and running, had to edit my pg_hba.conf to trust, I think my postgres db user had a password set.

geoffrey-eisenbarth commented 1 year ago

@palewire While writing tests I discovered a weird nuance about Django's "Model Constraints," it's now been addressed (and covered in testing).

The basic issue was that if a constraint is specified as

class Meta:
  constraints = [
    models.UniqueConstraint(
      fields=['field1', 'field2'],
      include=['field3'],  # This prevents a real psql "constraint" from being created
      name='my_constraint',
    )
  ],

then Django doesn't actually create a PSQL constrained named "my_constraint", it creates a UNIQUE INDEX, so doing something like ON CONFLICT ON CONSTRAINT "my_constraint" DO... will fail because there is no PSQL constraint named "my_constraint."

However, it seems in PSQL documentation for INSERT they recommend not naming constraints directly, but rather to let PSQL figure it out by just naming the relevant columns, so I've done that and things look good, all tests passing.

Going to look into how to update the documentation next. Is there anything obvious next steps I'm missing? Thanks!

geoffrey-eisenbarth commented 1 year ago

@palewire Is it possible for me to re-run the GitHub actions? The PR is passing tests and flake8 locally. Not sure how to progress.

geoffrey-eisenbarth commented 1 year ago

@palewire Can I get an update on this? It's been working well on my machine for awhile now, hoping to use it in production soon. Is there anything else I can do to move this forward?

Thanks!

geoffrey-eisenbarth commented 1 year ago

After a discussion awhile back on the postgresql Slack, it was recommended I add the IS DISTINCT FROM clause to avoid "updating" rows that didn't actually change.

geoffrey-eisenbarth commented 6 months ago

@palewire Any movement for this? I'm willing to fix any conflicts with the current version. If it's your preference not to support upsert, or do it a different way, could you let me know?

Thanks!

geoffrey-eisenbarth commented 1 week ago

@palewire Just giving you another ping. If you want me to let this go, just let me know. Thanks!