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

Add Parameters to the Output of Dry Run Migrations #477

Merged
merged 3 commits into from Jul 5, 2016

Conversation

chrisguitarguy
Copy link
Contributor

@chrisguitarguy chrisguitarguy commented Jul 5, 2016

Closes #186

Right now this looks like...

    -> INSERT INTO whatever VALUES (?, ?) with parameters (one, two)

with numeric indices in the param array. If there are string indices, those names are included in the output (under the assumption that they match placeholders in the SQL).

    -> INSERT INTO whatever VALUES (:one, :two) with parameters (:one => one, :two => two)

The DBAL type classes are used to convert the values to something suitable for output (specifically Type::convertToDatabaseValue.

SQL statements without any params look the same as they do currently.

This is probably a good enough first pass at this, but it's not going to protect users from doing something wonky like...

$this->addSql('INSERT INTO whatever VALUES (?)', [new \stdClass]);

and causing it the dryrun fail (granted the migration itself would fail anyway).

In other words: there's not a lot of validation. But that's not necessarily a bad thing. Types aren't validated to make sure they exist before fetching them with Type::getType(...), but that's something thay may help a user catch bugs.

This includes some failing tests for parameters being output with the
SQL statements.
@mikeSimonson
Copy link
Contributor

@chrisguitarguy I like this. Can you add a test with a datetime being passed ?

Including datetime and array, but setup to add more if necessary.
@chrisguitarguy
Copy link
Contributor Author

Added, @mikeSimonson. Also added a test for an array type. The test is setup to be able to easily add more types if necessary.

@mikeSimonson
Copy link
Contributor

@chrisguitarguy Thanks

@mikeSimonson mikeSimonson merged commit 5df0c51 into doctrine:master Jul 5, 2016
@mikeSimonson mikeSimonson added this to the 1.5 milestone Jul 5, 2016
@chrisguitarguy chrisguitarguy deleted the dryrun_params branch July 6, 2016 00:30
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

2 participants