Skip to content

fix: Forge::modifyColumn() for Postgre handler - #9676

Merged
michalsn merged 2 commits into
codeigniter4:developfrom
michalsn:fix/modifyColumn-postgre
Aug 15, 2025
Merged

fix: Forge::modifyColumn() for Postgre handler#9676
michalsn merged 2 commits into
codeigniter4:developfrom
michalsn:fix/modifyColumn-postgre

Conversation

@michalsn

@michalsnmichalsn commented Aug 12, 2025

Copy link
Copy Markdown
Member

Description
This PR fixes a bug in Postgre where changing a column's default value via Config\Database\Forge::modifyColumn() would result in incorrect SQL syntax.

Fixes#9671

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@michalsn
michalsn marked this pull request as draft August 12, 2025 19:09
@michalsnmichalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Aug 12, 2025
@michalsn

michalsn commented Aug 13, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@paulbalandan, given the recent testing challenges, do you have any thoughts on why this only happens with OCI8 and SQLSRV? Interestingly, migration locks (#9660) follow the same approach of creating a table for just one test case, but they don't seem to have these issues... unless I rebase - then it might be the same (but I'm not ready to test that yet) 😅

@paulbalandan

paulbalandan commented Aug 13, 2025

Copy link
Copy Markdown
Member

The test fails so the cleanup isn't reached causing test_stores to remain in the migration table. Though I'm not sure why your tests fail for those two DB only.

@michalsn

michalsn commented Aug 13, 2025

Copy link
Copy Markdown
MemberAuthor

Idk what I was thinking... I totally missed the errors... sorry 🤦

@michalsnmichalsn added the database Issues or pull requests that affect the database layer label Aug 13, 2025
@michalsn
michalsnforce-pushed the fix/modifyColumn-postgre branch from 14c5562 to 4721884CompareAugust 14, 2025 06:54
@michalsn
michalsn marked this pull request as ready for review August 14, 2025 07:10
neznaika0
neznaika0 reviewed Aug 14, 2025
View reviewed changes

@neznaika0neznaika0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm reviewing all the PR, but I don't know much for approval. It looks good.

@michalsn
michalsn merged commit 031d5c6 into codeigniter4:developAug 15, 2025
84 of 85 checks passed
@michalsn

michalsn commented Aug 15, 2025

Copy link
Copy Markdown
MemberAuthor

Thank you @paulbalandan and @neznaika0

@michalsn
michalsn deleted the fix/modifyColumn-postgre branch August 20, 2025 06:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulbalandanpaulbalandanpaulbalandan approved these changes
+1 more reviewer
@neznaika0neznaika0neznaika0 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bugVerified issues on the current code behavior or pull requests that will fix themdatabaseIssues or pull requests that affect the database layer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: duplicate the "DEFAULT" when trying to migrate

3 participants

@michalsn@paulbalandan@neznaika0