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

Apply assets filter on sequences #1328

Open
wants to merge 1 commit into
base: 3.8.x
Choose a base branch
from

Conversation

ottaviano
Copy link

@ottaviano ottaviano commented Feb 25, 2023

Q A
Type improvement
BC Break no
Fixed issues

Summary

Asset filter is used for tables and sequences on fromSchema, but only for tables on toSchema. It generates CREATE SEQUENCE migration query. This PR applies the filter for sequences on toSchema.

@derrabus derrabus changed the base branch from 3.6.x to 3.8.x March 6, 2024 14:06
@ottaviano ottaviano force-pushed the apply-filter-on-sequences branch 2 times, most recently from c7d4228 to 5a3be28 Compare June 1, 2024 20:23
@SenseException
Copy link
Member

Thank you for the PR, @ottaviano. Did you run into a specific problem that resulted into this change?

@ottaviano
Copy link
Author

@SenseException,
I have a partitioned table project_result_item with 3 sub-tables: project_result_item_0, project_result_item_1, and project_result_item_2.
When I run the doctrine:migration:diff command, it tries to drop the 3 sub-tables.

The "solution" I found is to completely ignore the project_result_item table with schema_filter, but Doctrine migration generates the sequence for the ignored table at every diff :
CREATE SEQUENCE project_result_item_id_seq INCREMENT BY 1 MINVALUE 1 START 1

This happens because the filter does not apply to sequences during the toSchema phase.

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

Please put the test for your changes into its own test case instead of mixing them with the one with the table name.

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.

2 participants