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

Fixed MigrationManager to use queries and charset settings #697

Merged
merged 1 commit into from Jun 6, 2013

Conversation

rozwell
Copy link
Contributor

@rozwell rozwell commented May 28, 2013

Before this PR more advanced migrations might fail or cause data issues when specific database configuration was used.
Mainly custom charset or collation.

See: propelorm/sfPropelORMPlugin#204

@willdurand
Copy link
Contributor

Wow, can't really say anything on this.. @propelorm who uses migrations to confirm the fix?

@rozwell
Copy link
Contributor Author

rozwell commented May 29, 2013

I do ;)
We encountered encoding issues during more advanced migrations and it turns out that Propel::initConnection() executes queries like SET NAMES utf8 or adds charset parameter to the dsn (etc) - raw new PDO() does not.

@havvg
Copy link
Member

havvg commented May 29, 2013

Looks good to me.

Used a lot migrations, but never run into the issue, probably because we set everything to UTF-8 on server configuration.

@rozwell
Copy link
Contributor Author

rozwell commented May 29, 2013

@havvg we're using UTF-8 too but with utf8_polish_ci collation. AFAIR without SET NAMES UTF-8 queries didn't work correctly.

@staabm
Copy link
Member

staabm commented May 29, 2013

@rozwell you need the SET NAMES statement only when the server itself does not default to a UTF-8 charset, see e.g. http://dev.mysql.com/doc/refman/5.0/en/server-system-variables.html#sysvar_character_set_server

(and it is not overwritten by a database/table/connnection collation)

@rozwell
Copy link
Contributor Author

rozwell commented May 29, 2013

I'm aware of that. But that's theory. I'm always using utf8 and somehow It didn't work correctly. Maybe because of collation, doesn't matter - I've got this working. Just wanted to share the fix with others :)

willdurand added a commit that referenced this pull request Jun 6, 2013
Fixed MigrationManager to use queries and charset settings
@willdurand willdurand merged commit 9c19ce6 into propelorm:master Jun 6, 2013
@willdurand
Copy link
Contributor

thanks!

@willdurand
Copy link
Contributor

@rozwell could you port this into Propel2?

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.

None yet

5 participants