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

Migration & MySql Query addCheck() and dropCheck() #19881

Merged
merged 18 commits into from
May 22, 2024
Merged

Conversation

bobonov
Copy link
Contributor

@bobonov bobonov commented Jul 5, 2023

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues #19817, #19819

@what-the-diff
Copy link

what-the-diff bot commented Jul 5, 2023

PR Summary

  • Check Constraint Management in Migration Class
    In the Migration file (framework/db/Migration.php), two new methods are introduced. The addCheck method is created to generate a SQL command that adds a rule - a check constraint, to an existing database table. Similarly, the dropCheck method is introduced to compose a SQL command for removing any check constraint from a database table.

  • Removal of Unsupported Methods in MySQL Query Builder
    In the MySQL Query Builder file (framework/db/mysql/QueryBuilder.php), addCheck and dropCheck methods are discarded. The motivation behind this action is that MySQL does not support these operations, hence removing them improves the overall robustness and reliability of this part of the system.

@samdark samdark added this to the 2.0.49 milestone Jul 7, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

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

Project coverage is 60.49%. Comparing base (391997a) to head (bd73df5).
Report is 1 commits behind head on master.

Current head bd73df5 differs from pull request most recent head 2ead4f3

Please upload reports for the commit 2ead4f3 to get more accurate results.

Files Patch % Lines
framework/db/mysql/Schema.php 0.00% 23 Missing ⚠️
framework/db/Migration.php 0.00% 8 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #19881       +/-   ##
=============================================
+ Coverage     18.65%   60.49%   +41.84%     
- Complexity    11379    11383        +4     
=============================================
  Files           430      430               
  Lines         37071    37103       +32     
=============================================
+ Hits           6915    22445    +15530     
+ Misses        30156    14658    -15498     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@samdark samdark requested review from a team July 18, 2023 14:55
framework/db/Migration.php Outdated Show resolved Hide resolved
@bizley bizley modified the milestones: 2.0.49, 2.0.50 Aug 29, 2023
@terabytesoftw
Copy link
Member

hi, @bobonov, would have time to apply the suggestions, to finish the PR,

Thks.

@terabytesoftw
Copy link
Member

Hi, add line in changelog,

Thks,

@bizley bizley modified the milestones: 2.0.49.1, 2.0.50 Oct 5, 2023
@mtangoo
Copy link
Contributor

mtangoo commented Feb 15, 2024

@bobonov
Thanks for the PR. Would you please work on the comments, notably change log entries, so that the PR can be merged?

@samdark samdark modified the milestone: 2.0.50 May 16, 2024
Copy link
Contributor

@rob006 rob006 left a comment

Choose a reason for hiding this comment

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

I think we need to enable MySQL tests for this feature:

public function testAddDropCheck()

framework/db/Migration.php Outdated Show resolved Hide resolved
@terabytesoftw
Copy link
Member

I think we need to enable MySQL tests for this feature:

public function testAddDropCheck()

done.

@terabytesoftw terabytesoftw marked this pull request as draft May 18, 2024 16:03
@terabytesoftw
Copy link
Member

terabytesoftw commented May 18, 2024

This PR needs to be updated, since for Yii2 to correctly support addCheck() and dropCheck() in MySQL, loadTableChecks() must be implemented for MySQL 8.0.16 or higher, so it is in progress.

@mtangoo
Copy link
Contributor

mtangoo commented May 18, 2024

The PR title and changelog need to be updated too, since it does more than add/drop check

@terabytesoftw
Copy link
Member

Please if you can take a look, all the tests work,

Thks.

@rob006 @bizley @darkdef

@mtangoo
Copy link
Contributor

mtangoo commented May 20, 2024

@rob006 @darkdef @rhertogh can you review this PR? Would like to see it merged before end of this week, if we can make that happen!

@samdark samdark marked this pull request as ready for review May 22, 2024 14:34
@mtangoo mtangoo merged commit 9975597 into yiisoft:master May 22, 2024
70 of 71 checks passed
@mtangoo
Copy link
Contributor

mtangoo commented May 22, 2024

Thanks everyone and especially @bobonov and @terabytesoftw for your work on the PR

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.

7 participants