piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

Add support for composite unique constraint in auto migration

Open atkei opened this issue 10 months ago • 13 comments

Related to #172, #175 and based on #957.

Add UniqueConstraint to support composite unique constraint in auto migration.

from piccolo.columns import Text
from piccolo.constraint import UniqueConstraint
from piccolo.table import Table


class Band(Table):
    name = Text(required=True)
    label = Text(required=True)
    unique_name_label = UniqueConstraint(["name", "label"])

Some work remains such as adding doc and testing, but I would like to have feedback on my proposal first.

atkei avatar Apr 12 '24 12:04 atkei

Codecov Report

Attention: Patch coverage is 95.85253% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 89.72%. Comparing base (363d683) to head (07f8b94). Report is 1 commits behind head on master.

Files Patch % Lines
piccolo/apps/migrations/auto/schema_differ.py 92.85% 3 Missing :warning:
piccolo/apps/migrations/auto/migration_manager.py 97.36% 2 Missing :warning:
piccolo/apps/migrations/auto/diffable_table.py 95.00% 1 Missing :warning:
piccolo/apps/migrations/auto/schema_snapshot.py 80.00% 1 Missing :warning:
piccolo/constraint.py 96.96% 1 Missing :warning:
piccolo/table.py 90.90% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #984      +/-   ##
==========================================
- Coverage   92.78%   89.72%   -3.07%     
==========================================
  Files         108      109       +1     
  Lines        8182     8399     +217     
==========================================
- Hits         7592     7536      -56     
- Misses        590      863     +273     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 12 '24 15:04 codecov-commenter

@dantownsend I hope it's not a problem, but I have approved and run the workflow for this PR. I have already tried these changes in the @atkei local branch and in my case everything works great as I wrote in this comment. Can you try and review this PR. Thanks.

sinisaos avatar Apr 12 '24 15:04 sinisaos

@sinisaos Awesome thanks - feel free to run the CI whenever you want.

@atkei Thanks a lot for this - looks great!

dantownsend avatar Apr 12 '24 15:04 dantownsend

@sinisaos @dantownsend - could you make a hint - when it would be merged? Would it mean new version/release?

AlexanderMakarov avatar May 01 '24 08:05 AlexanderMakarov

@AlexanderMakarov I'll try my best to review it properly this week.

dantownsend avatar May 01 '24 08:05 dantownsend

Currenty looking for this funtionality - would appreciate it someone on the team would be able to review!

MoSheikh avatar May 20 '24 18:05 MoSheikh

@sinisaos @dantownsend - do you have some plans to deliver this functionality soon? Thank you in advance.

AlexanderMakarov avatar Aug 08 '24 11:08 AlexanderMakarov

@AlexanderMakarov Sorry, but you have to wait for @dantownsend response.

sinisaos avatar Aug 08 '24 11:08 sinisaos

Yes, it's definitely high on my radar. I need to try and get some improvements done to Piccolo Admin, but this is high on my list. Thanks for the patience.

dantownsend avatar Aug 08 '24 15:08 dantownsend

@dantownsend Is this released?

devsarvesh92 avatar Aug 29 '24 13:08 devsarvesh92

@dantownsend is there any workaround to acheive this?

devsarvesh92 avatar Aug 29 '24 13:08 devsarvesh92

@devsarvesh92 For now, the workaround is to use raw sql (in migrations or script) to add a composite unique constraint, as described here.

sinisaos avatar Aug 29 '24 14:08 sinisaos