Fixed bug that the command migrate:fresh can't drop postgres tables by default.#7447
Merged
limingxinleo merged 6 commits intohyperf:masterfrom Aug 2, 2025
Merged
Conversation
Member
|
Please update changelog. |
Contributor
Author
|
@sy-records Done |
huangdijia
previously approved these changes
Jul 15, 2025
Member
huangdijia
left a comment
There was a problem hiding this comment.
Code Review: Approve ✅
Overview
This PR fixes a bug where PostgreSQL tables weren't being dropped during migrate:fresh operations when no schema was configured. The solution adds a getSchemas() method that defaults to ['public'], aligning with Laravel's behavior.
Analysis
✅ Code Quality & Style
- Clean, readable implementation following existing patterns
- Proper PHPDoc documentation
- Consistent with codebase conventions
✅ Solution Approach
- Addresses root cause: empty schema array passed to
compileGetAllTables() - Centralized schema resolution in
getSchemas()method - Applied consistently to both
getAllTables()andgetAllViews()
✅ Technical Implementation
- Appropriate method placement as protected
- Simple and robust logic
- Good reusability between methods
✅ Security & Performance
- No security risks identified
- Minimal performance impact
Minor Considerations
- Consider adding unit tests for default schema behavior
- Schema assumption of
publicis standard for PostgreSQL
Recommendation
Approve - Well-implemented fix that cleanly solves the reported issue and follows established patterns.
92c0ff1
getAllTables if no schema providedmigrate:fresh can't drop postgres tables by default.
limingxinleo
approved these changes
Aug 2, 2025
limingxinleo
approved these changes
Aug 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running
php bin/hyperf.php migrate:freshwith PostgreSQL, tables are not dropped if no schema is configured in the database config. This happens becausePostgresBuilder::getAllTables()passes an empty array tocompileGetAllTables()when no schema is configured, which results in no tables being returned.This PR adds a
getSchemas()method that defaults to['public']when no schema is configured, matching Laravel's behavior:https://github.com/laravel/framework/blob/0bbe0022ea60de20fabaed520c00c84689b083ee/src/Illuminate/Database/Schema/PostgresBuilder.php#L237