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

Propel diff task does not honor column order on MySQL #274

Closed
Shyru opened this issue Feb 1, 2012 · 4 comments
Closed

Propel diff task does not honor column order on MySQL #274

Shyru opened this issue Feb 1, 2012 · 4 comments

Comments

@Shyru
Copy link

Shyru commented Feb 1, 2012

When new columns are added to a table from one version to the other, the propel migration will add all columns to the end of the table.
For MySQL this is fixable, because MySQL supports an ALTER TABLE ADD COLUMN abc AFTER Xy Syntax.
I fixed this by reimplementing getAddColumnDDL() and getAddColumnsDDL() in MysqlPlatform.
Pull request incoming...

@Shyru
Copy link
Author

Shyru commented Feb 1, 2012

Here is the link to the pull-request implementing a fix:
#275

@fzaninotto
Copy link
Member

Respecting the XML order in the SQL migrations is not a requirement, since Propel always asks for columns explicitly in every request.

That being said, if you need to have the same columns in XML and SQL, you can always edit the generated migrations before executing them.

@Shyru
Copy link
Author

Shyru commented Feb 19, 2012

For us it is a requirement, as we use the column order to automatically generate forms to edit models. So for us its important to have the correct order. And when we have to rewrite all migrations by hand this feature is not of great use for us.
So you tell me that we have to apply this patch every time we update Propel?

@nihongomaster
Copy link

This fails when attempting to use a custom query/prepared statement.

If by doing SELECT * FROM

WHERE <table.id> NOT IN (SUBSELECT)

And then using a formatter to format it into objects, the column order is wrong, therefore it breaks.

This needs to be reopen to handle this edge case.

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 a pull request may close this issue.

2 participants