-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
Show the sql statement parameters if they exist #513
Conversation
@@ -407,7 +407,8 @@ private function executeRegisteredSql($dryRun = false, $timeAllQueries = false) | |||
$this->outputWriter->write(' <comment>-></comment> ' . $query); | |||
$this->connection->executeQuery($query); | |||
} else { | |||
$this->outputWriter->write(sprintf(' <comment>-</comment> %s (with parameters)', $query)); | |||
$parameters = implode(', ', $this->params[$key]); | |||
$this->outputWriter->write(sprintf(' <comment>-</comment> %s (%s)', $query, $parameters)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole bit (including the if/else
check for the params) could be replaced with $this->outputSqlQuery($key, $query)
.
I added that in #477 -- uses the doctrine types system to convert the params into something printable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better indeed.
1f9e1ed
to
eff7d50
Compare
@chrisguitarguy Thanks for the review |
$this->connection->executeQuery($query); | ||
} else { | ||
$this->outputWriter->write(sprintf(' <comment>-</comment> %s (with parameters)', $query)); | ||
$this->outputSqlQuery($key, $query); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calls to $this->outputSqlQuery
could be moved above the if/else
block here.
eff7d50
to
3ebe6cd
Compare
@@ -403,11 +403,10 @@ private function executeRegisteredSql($dryRun = false, $timeAllQueries = false) | |||
foreach ($this->sql as $key => $query) { | |||
$queryStart = microtime(true); | |||
|
|||
$this->outputSqlQuery($key, $query); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will probably break migrations that use \PDO::*
or \Doctrine\DBAL\Connection::*
parameter types (see #528)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikeSimonson is this a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have been fixed by #560. I will try to add a test for that specific case.
This is implemented and covered by tests now. |
No description provided.