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

Modify SetColumnRemarks support for mysql. The original code does not support mysql modification notes #942

Merged
merged 6 commits into from
Jan 12, 2021

Conversation

zyzyzy523
Copy link

@zyzyzy523 zyzyzy523 commented Nov 28, 2019

It turns out that the sql statement generated by setColumnRemarks of the mysql database is ALTER TABLE cat.person COMMENT = 'A String';
Now modify it to ALTER TABLE cat.person MODIFY COLUMN id int COMMENT 'A String';

@SteveDonie
Copy link
Contributor

Thanks for the pull request. Can you please edit it so that all the comments are in English?
We will need to review this because of the change to the xsd and decide when it can be included.

@zyzyzy523 zyzyzy523 changed the title 修改SetColumnRemarks对mysql的支持。原代码不支持mysql修改备注 Modify SetColumnRemarks support for mysql. The original code does not support mysql modification notes Dec 4, 2019
@SteveDonie
Copy link
Contributor

Thanks for the update!

@SteveDonie SteveDonie added TypeBug and removed bugfix labels Mar 3, 2020
@datical-jenkins datical-jenkins changed the title Modify SetColumnRemarks support for mysql. The original code does not support mysql modification notes LB-38 ⁃ Modify SetColumnRemarks support for mysql. The original code does not support mysql modification notes Mar 4, 2020
@datical-jenkins datical-jenkins changed the title LB-38 ⁃ Modify SetColumnRemarks support for mysql. The original code does not support mysql modification notes Modify SetColumnRemarks support for mysql. The original code does not support mysql modification notes Mar 5, 2020
# Conflicts:
#	liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java
@codecov
Copy link

codecov bot commented Apr 19, 2020

Codecov Report

Merging #942 into master will increase coverage by 0.00%.
The diff coverage is 67.74%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #942   +/-   ##
=========================================
  Coverage     47.71%   47.72%           
- Complexity     7477     7482    +5     
=========================================
  Files           757      757           
  Lines         36278    36302   +24     
  Branches       6624     6628    +4     
=========================================
+ Hits          17311    17326   +15     
- Misses        16660    16666    +6     
- Partials       2307     2310    +3     
Impacted Files Coverage Δ Complexity Δ
...in/java/liquibase/change/core/AddColumnChange.java 54.68% <0.00%> (-0.44%) 31.00 <0.00> (ø)
.../java/liquibase/change/core/CreateTableChange.java 68.80% <0.00%> (ø) 44.00 <0.00> (ø)
...e/sqlgenerator/core/SetColumnRemarksGenerator.java 80.00% <60.00%> (-12.60%) 16.00 <0.00> (+1.00) ⬇️
.../liquibase/change/core/SetColumnRemarksChange.java 92.85% <100.00%> (+1.94%) 17.00 <4.00> (+4.00)
...base/statement/core/SetColumnRemarksStatement.java 100.00% <100.00%> (ø) 8.00 <3.00> (+2.00)
...re/src/main/java/liquibase/util/ISODateFormat.java 77.08% <0.00%> (-2.09%) 15.00% <0.00%> (-1.00%)
...-core/src/main/java/liquibase/util/ObjectUtil.java 37.43% <0.00%> (-0.56%) 35.00% <0.00%> (-1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update baee65d...0244507. Read the comment docs.

@ro-rah
Copy link

ro-rah commented Apr 21, 2020

Hi @zyzyzy523 ,

Thanks for your PR submission. Its been awhile since we heard from you. Did you need any help or further guidance around the changes @nvoxland requested?

Thanks!

Ronak

@ro-rah
Copy link

ro-rah commented May 11, 2020

@zyzyzy523 I will move this into Conditioning but size it RiskMed. Eng time will be needed to make the change requested by @nvoxland. Thanks for including tests and translations!

@ro-rah ro-rah added RiskMedium Changes that require more testing or that affect many different code paths. StatusConditioning and removed StatusDiscovery labels May 11, 2020
php-coder added a commit to php-coder/mystamps that referenced this pull request Sep 8, 2022
…instead of the tables) comments

In the commit 89ba68c (#1326) I used <setColumnRemarks> in order to
put comments to the fields. It has turned out that on MySQL, Liquibase put comments on the tables
instead of the fields (see liquibase/liquibase#942). After update of
Liquibase (#1526), it became possible to fix the bug on our data.

Fix #1408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DBMySQL IntegrationAny RiskMedium Changes that require more testing or that affect many different code paths. ThemeDBObjects TypeBug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setColumnRemarks Mysql execute sql : ALTER TABLE [tabeName] COMMENT = 'your comment';
8 participants