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

doctrine:migration:diff does not work if the table has an index with a functional part. #6153

Closed
wants to merge 10 commits into from

Conversation

prohalexey
Copy link

Q A
Type bug

Summary

if the table has an index with a functional part, command doctrine:migration:diff fails with error

In Index.php line 81:
                                                                               
  Doctrine\DBAL\Schema\Index::_addColumn(): Argument #1 ($column) must be of   
  type string, null given, called in /opt/core/vendor/doctrine/dbal/src/Schem  
  a/Index.php on line 72 

That's because column name for the functional part is null.

@derrabus
Copy link
Member

Thank you. Please note that we don't merge any changes without tests.

@prohalexey prohalexey marked this pull request as draft September 16, 2023 07:57
@derrabus derrabus marked this pull request as ready for review September 17, 2023 14:20
@derrabus derrabus marked this pull request as draft September 18, 2023 07:04
@prohalexey prohalexey marked this pull request as ready for review September 18, 2023 07:37
@derrabus derrabus changed the base branch from 3.6.x to 3.7.x September 26, 2023 21:56
@prohalexey
Copy link
Author

@derrabus What tests do you think will be suitable for this patch?

@derrabus
Copy link
Member

A test that reproduces your issue and fails without your change.

@derrabus derrabus changed the base branch from 3.7.x to 3.8.x October 13, 2023 14:35
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Please add functional tests. The tests you've added never hit a real database and thus don't tell us that your feature actually works.

src/Schema/AbstractSchemaManager.php Outdated Show resolved Hide resolved
src/Schema/Index.php Outdated Show resolved Hide resolved
@prohalexey prohalexey marked this pull request as draft October 16, 2023 12:22
@prohalexey prohalexey marked this pull request as ready for review October 17, 2023 15:19
@prohalexey
Copy link
Author

prohalexey commented Oct 18, 2023

@derrabus Could you give some advice or explanations about falling tests? I tried running tests on my local machine against postgresql and mysql and they passed. As I see, there is a lot of errors, and i don't see the correlation between the errors and my code. What could be a root cause of problem ?

Copy link

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

@github-actions github-actions bot added the Stale label Jan 17, 2024
Copy link

This pull request was closed due to inactivity.

@github-actions github-actions bot closed this Jan 24, 2024
@prohalexey prohalexey deleted the functional_part_of_index branch May 29, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants