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

fix(MariaDBTable): dont attempt to drop index twice #19783

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

sagarvora
Copy link
Collaborator

Prevent error when syncing columns that have both a unique index AND a non-unique one:

In [1]: frappe.db.sql("alter table `tabInstalled Application` add index app_name
   ...: _index(app_name)")
Out[1]: ()

In [2]: frappe.db.sql("alter table `tabInstalled Application` add unique index a
   ...: pp_name_index_(app_name)")
Out[2]: ()

In [3]: from frappe.database.mariadb.schema import MariaDBTable

In [4]: x = MariaDBTable("Installed Application")

In [5]: x.validate()

In [6]: x.sync()
Failed to alter schema using query: ALTER TABLE `tabInstalled Application` DROP INDEX `app_name_index_`, DROP INDEX `app_name_index`, DROP INDEX `app_name_index_`, DROP INDEX `app_name_index`

--- snip traceback ---

Issue encountered when upgrading a site to version 14.

@sagarvora sagarvora requested a review from a team as a code owner January 26, 2023 06:16
@sagarvora sagarvora requested review from phot0n and removed request for a team January 26, 2023 06:16
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Jan 26, 2023
@sagarvora sagarvora added backport version-13-hotfix backport version-14-hotfix backport to version 14 and removed add-test-cases Add test case to validate fix or enhancement labels Jan 26, 2023
@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Merging #19783 (e8e1ed0) into develop (fb9ae08) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #19783      +/-   ##
===========================================
- Coverage    62.75%   62.75%   -0.01%     
===========================================
  Files          754      754              
  Lines        71764    71764              
  Branches      6134     6134              
===========================================
- Hits         45036    45035       -1     
- Misses       23252    23253       +1     
  Partials      3476     3476              
Flag Coverage Δ
server 68.70% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ankush
Copy link
Member

ankush commented Jan 26, 2023

that have both a unique index AND a non-unique one

We prevent this from happening right? 🥲

# unique is automatically an index
if d.unique:
d.search_index = 0

@sagarvora
Copy link
Collaborator Author

We prevent this from happening right?

This is the actual query encountered during migration:

ALTER TABLE `tabBank Account` DROP INDEX `account_name_2`, DROP INDEX `account_name`, DROP INDEX `account_name_2`, DROP INDEX `account_name`

Not sure if it is a user-specific issue.

@ankush
Copy link
Member

ankush commented Jan 26, 2023

@sagarvora looks like it's this: #19291

Anyway this PR makes sense. Another alternative: run one alter query at time.

@ankush ankush merged commit db9b25e into frappe:develop Jan 27, 2023
@ankush ankush deleted the dont-drop-twice branch January 27, 2023 03:57
mergify bot pushed a commit that referenced this pull request Jan 27, 2023
sagarvora added a commit that referenced this pull request Jan 27, 2023
…-19783

fix(MariaDBTable): dont attempt to drop index twice (backport #19783)
sagarvora added a commit that referenced this pull request Jan 27, 2023
ankush pushed a commit that referenced this pull request Jan 27, 2023
(cherry picked from commit db9b25e)

Co-authored-by: Sagar Vora <sagar@resilient.tech>
frappe-pr-bot pushed a commit that referenced this pull request Jan 30, 2023
# [13.49.0](v13.48.0...v13.49.0) (2023-01-30)

### Bug Fixes

* add freeze message for bulk delete ([486df4c](486df4c))
* **i18n:** Datepicker Turkish translations ([#19777](#19777)) ([#19830](#19830)) ([5ba34b4](5ba34b4))
* incorrect link when std field has problem (backport [#19744](#19744)) ([#19764](#19764)) ([30729a2](30729a2))
* **MariaDBTable:** dont attempt to drop index twice ([#19783](#19783)) ([#19801](#19801)) ([1b94c62](1b94c62))
* use count instead of concatenated docnames ([d8d7373](d8d7373))

### Features

* Audit hooks report (backport [#19780](#19780)) ([#19827](#19827)) ([361e250](361e250))
* better freeze message ([7f46807](7f46807))
* let users modify hook resolution order (backport [#19653](#19653)) ([#19774](#19774)) ([76e0974](76e0974)), closes [#19672](#19672)
frappe-pr-bot pushed a commit that referenced this pull request Jan 30, 2023
# [14.25.0](v14.24.0...v14.25.0) (2023-01-30)

### Bug Fixes

* add freeze message for bulk delete ([2a42036](2a42036))
* assertAlmostEqual with precision ([#19794](#19794)) ([9f7c4e0](9f7c4e0))
* Convert doctype name to string ([#19832](#19832)) ([#19834](#19834)) ([a45f31d](a45f31d))
* correct exit code on missing app failure ([#19676](#19676)) ([#19770](#19770)) ([f6139a4](f6139a4))
* **i18n:** Datepicker Turkish translations ([#19777](#19777)) ([#19831](#19831)) ([3e91fb1](3e91fb1))
* incorrect link when std field has problem (backport [#19744](#19744)) ([#19763](#19763)) ([4593bb9](4593bb9))
* **MariaDBTable:** dont attempt to drop index twice ([#19783](#19783)) ([67f80c6](67f80c6))
* Password strength check for long passwords (backport [#19756](#19756)) ([#19765](#19765)) ([a6315f9](a6315f9))
* respect disable sidebar stats on list view ([#19795](#19795)) ([5f57816](5f57816))
* sanitize traceback for common secrets ([#19805](#19805)) ([#19806](#19806)) ([ae6f2b1](ae6f2b1))
* use count instead of concatenated docnames ([06948d1](06948d1))

### Features

* Audit hooks report (backport [#19780](#19780)) ([#19828](#19828)) ([99bdf34](99bdf34))
* better freeze message ([c03f9e7](c03f9e7))
stephenBDT pushed a commit to alias/frappe that referenced this pull request Feb 7, 2023
stephenBDT pushed a commit to alias/frappe that referenced this pull request Feb 7, 2023
# [14.25.0](frappe/frappe@v14.24.0...v14.25.0) (2023-01-30)

### Bug Fixes

* add freeze message for bulk delete ([2a42036](frappe@2a42036))
* assertAlmostEqual with precision ([frappe#19794](frappe#19794)) ([9f7c4e0](frappe@9f7c4e0))
* Convert doctype name to string ([frappe#19832](frappe#19832)) ([frappe#19834](frappe#19834)) ([a45f31d](frappe@a45f31d))
* correct exit code on missing app failure ([frappe#19676](frappe#19676)) ([frappe#19770](frappe#19770)) ([f6139a4](frappe@f6139a4))
* **i18n:** Datepicker Turkish translations ([frappe#19777](frappe#19777)) ([frappe#19831](frappe#19831)) ([3e91fb1](frappe@3e91fb1))
* incorrect link when std field has problem (backport [frappe#19744](frappe#19744)) ([frappe#19763](frappe#19763)) ([4593bb9](frappe@4593bb9))
* **MariaDBTable:** dont attempt to drop index twice ([frappe#19783](frappe#19783)) ([67f80c6](frappe@67f80c6))
* Password strength check for long passwords (backport [frappe#19756](frappe#19756)) ([frappe#19765](frappe#19765)) ([a6315f9](frappe@a6315f9))
* respect disable sidebar stats on list view ([frappe#19795](frappe#19795)) ([5f57816](frappe@5f57816))
* sanitize traceback for common secrets ([frappe#19805](frappe#19805)) ([frappe#19806](frappe#19806)) ([ae6f2b1](frappe@ae6f2b1))
* use count instead of concatenated docnames ([06948d1](frappe@06948d1))

### Features

* Audit hooks report (backport [frappe#19780](frappe#19780)) ([frappe#19828](frappe#19828)) ([99bdf34](frappe@99bdf34))
* better freeze message ([c03f9e7](frappe@c03f9e7))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants