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

Output "may lose settings" warning on mysql/mariadb for more impacted change types #3045

Merged
merged 5 commits into from Jul 28, 2022

Conversation

nvoxland
Copy link
Contributor

@nvoxland nvoxland commented Jul 6, 2022

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

MySQL/MariaDB's alter table ... modify column syntax requires a complete redefinition of the column when modifying a column, such as setting a comment, changing a datatype, etc.

The the setColumnRemarks, addDefaultValue, etc change does not support all possible column features, only the data type so anything else (like nullability etc) will be lost.

This PR expands the existing warning we had for modifyDataType, since that is all we can do for this.

Things to be aware of

  • Only impacts Mysql/MariaDB.
  • Adds warnings to addDefaultValue, setColumnRemarks, addNotNull, and dropNotNull changes.

Things to worry about

  • Nothing

@nvoxland nvoxland changed the title Output warning o Output "may loose settings" warning on mysql/mariadb for setColumnRemarks Jul 6, 2022
@nvoxland nvoxland changed the title Output "may loose settings" warning on mysql/mariadb for setColumnRemarks Output "may loose settings" warning on mysql/mariadb for more impacted change types Jul 6, 2022
@github-actions
Copy link

github-actions bot commented Jul 6, 2022

Unit Test Results

  4 620 files  ±0    4 620 suites  ±0   32m 8s ⏱️ - 5m 31s
  4 594 tests ±0    4 379 ✔️ ±0     215 💤 ±0  0 ±0 
54 300 runs  ±0  49 280 ✔️ ±0  5 020 💤 ±0  0 ±0 

Results for commit 612ca9d. ± Comparison against base commit 65c65ba.

♻️ This comment has been updated with latest results.

@kataggart kataggart added this to To Do in Conditioning++ via automation Jul 6, 2022
@nvoxland nvoxland requested a review from suryaaki2 July 15, 2022 17:53
@nvoxland nvoxland removed their assignment Jul 26, 2022
@FBurguer
Copy link

FBurguer commented Jul 27, 2022

For this PR tested if every change type listed before had a warning msg on MySQL and MariaDB:

  • addDefaultValue
    MySQL: PASS
    MariaDB: PASS

  • setColumnRemarks
    MySQL: PASS
    MariaDB: PASS

  • addNotNull
    MySQL: PASS
    MariaDB: PASS

  • dropNotNull
    MySQL: PASS
    MariaDB: PASS

    Test Environment:
    OS: Windows 10
    Java 8
    MySQL v8.0.27
    MariaDB v10.5

@FBurguer FBurguer closed this Jul 27, 2022
Conditioning++ automation moved this from To Do to Done Jul 27, 2022
@FBurguer FBurguer reopened this Jul 27, 2022
Conditioning++ automation moved this from Done to To Do Jul 27, 2022
@nvoxland nvoxland merged commit a36a972 into master Jul 28, 2022
Conditioning++ automation moved this from To Do to Done Jul 28, 2022
@nvoxland nvoxland deleted the warn-on-addComment-mysql branch July 28, 2022 05:08
@kataggart kataggart changed the title Output "may loose settings" warning on mysql/mariadb for more impacted change types Output "may lose settings" warning on mysql/mariadb for more impacted change types Jul 28, 2022
@kataggart kataggart added this to the NEXT milestone Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants