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

Drop constraint quota_definitions_name_key #3923

Closed

Conversation

kathap
Copy link
Contributor

@kathap kathap commented Aug 8, 2024

On the name column in quota_defintions there were two unique constraints/indexes. One unique index with name qd_name_index, which is similar in postgres and mysql, and then an unique constraint on column name with name in postgres=quota_definitions_name_key and name in mysql=name. These index/constraint are created during table creation:

    create_table :quota_definitions do
      VCAP::Migration.common(self, :qd)

      String :name, null: false, unique: true, case_insensitive: true
      Boolean :non_basic_services_allowed, null: false
      Integer :total_services, null: false
      Integer :memory_limit, null: false

      index :name, unique: true
    end

The unique: true in line String :name, null: false, unique: true, case_insensitive: true creates the unique constraint.
Since there is an unique index created on column name index :name, unique: true (later renamed to qd_name_index), we don't need the unique constraint. Dropping it with this migration.

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@kathap kathap force-pushed the remove-duplilcated-unique-constraint-from-quota-definitions branch from cacc352 to bf09cfd Compare August 8, 2024 12:40
kathap added a commit that referenced this pull request Aug 8, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
Copy link
Contributor

@svkrieger svkrieger left a comment

Choose a reason for hiding this comment

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

Can you please add tests for the migration to make sure it does what it should and that it is idempotent?
Example: https://github.com/cloudfoundry/cloud_controller_ng/blob/main/spec/migrations/20240820070742_add_jobs_user_guid_state_index_spec.rb

@kathap kathap force-pushed the remove-duplilcated-unique-constraint-from-quota-definitions branch 2 times, most recently from 14c2f79 to d4690fd Compare August 30, 2024 09:57
On the name column in quota_defintions there were two unique
constraints/indexes. One unique index with name qd_name_index, which is
similar in postgres and mysql, and then an unique constraint on column
name with name in postgres=quota_definitions_name_key and name in
mysql=name.
Since the unique index already exists, we don't need the unique
constraint. Dropping it with this migration.
@kathap kathap force-pushed the remove-duplilcated-unique-constraint-from-quota-definitions branch 2 times, most recently from 3c1d718 to b310558 Compare September 2, 2024 08:55
@kathap kathap force-pushed the remove-duplilcated-unique-constraint-from-quota-definitions branch from b310558 to a061f4a Compare September 2, 2024 09:07
@kathap kathap marked this pull request as draft September 4, 2024 07:40
@kathap kathap force-pushed the remove-duplilcated-unique-constraint-from-quota-definitions branch from 0848fd3 to aa3a252 Compare September 4, 2024 10:05
@kathap kathap closed this Sep 5, 2024
@kathap
Copy link
Contributor Author

kathap commented Sep 5, 2024

corrupt branch

@kathap kathap deleted the remove-duplilcated-unique-constraint-from-quota-definitions branch September 5, 2024 07:14
svkrieger pushed a commit that referenced this pull request Oct 16, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
svkrieger pushed a commit that referenced this pull request Oct 16, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
svkrieger pushed a commit that referenced this pull request Oct 17, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
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