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

Split Migration 0004 into two migrations #1196

Closed

Conversation

abu-siddique-shipsy
Copy link

@abu-siddique-shipsy abu-siddique-shipsy commented Aug 14, 2022

Citus Postgres DB doesnt allow adding of new field with constrains on it. Example Error attached

You can issue each command separately such as ALTER TABLE oauth2_provider_accesstoken ADD COLUMN id_token data_type; ALTER TABLE oauth2_provider_accesstoken ADD CONSTRAINT constraint_name CHECK (check_expression);

Fixes #

Description of the Change

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@abu-siddique-shipsy abu-siddique-shipsy changed the title Update 0004_auto_20200902_2022.py Split Migration 0004 into two migrations Aug 15, 2022
@codecov
Copy link

codecov bot commented May 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.54%. Comparing base (560f84d) to head (bb5224f).
Report is 3 commits behind head on master.

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1196      +/-   ##
==========================================
- Coverage   97.56%   97.54%   -0.02%     
==========================================
  Files          32       32              
  Lines        2132     2120      -12     
==========================================
- Hits         2080     2068      -12     
  Misses         52       52              

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

Copy link
Contributor

@dopry dopry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Citus postgres isn't technically a supported database. That said I don't see a reason not to do our best to support it.

However we need to document why we're doing this here. Please add comment explaining why, with links to Citus DB documentation that demonstrates what is not supported and the specific error that you get from Citus if this is not done in 2 steps.

Citus Postgres DB doesnt allow adding of new field with constrains on it.  Example Error attached

You can issue each command separately such as ALTER TABLE oauth2_provider_accesstoken ADD COLUMN id_token data_type; ALTER TABLE oauth2_provider_accesstoken ADD CONSTRAINT constraint_name CHECK (check_expression);
@n2ygk
Copy link
Member

n2ygk commented May 7, 2024

@abu-siddique-shipsy will you be doing this?

Citus postgres isn't technically a supported database. That said I don't see a reason not to do our best to support it.

However we need to document why we're doing this here. Please add comment explaining why, with links to Citus DB documentation that demonstrates what is not supported and the specific error that you get from Citus if this is not done in 2 steps.

@n2ygk
Copy link
Member

n2ygk commented May 20, 2024

@abu-siddique-shipsy Have you opened an issue against Citus that shows this error that makes it non-compliant with Django Postgres support? It seems like that's where the appropriate compatibility fix should live given that DOT is not the only place where migrations like 0004 happen.

Also you say "Example Error attached" above but I don't see the attachment.

@n2ygk
Copy link
Member

n2ygk commented May 20, 2024

closing as stale. Feel free to reopen with the requested changes.

@n2ygk n2ygk closed this May 20, 2024
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.

3 participants