MySql Schema::renameTable is wrongly returning a value

I can try to fix this, hopefully, it's not complicated.

Patch included.

Code change in above patch works fine.

Hi all,

I am not very familiar with the tests, but I guess you wanted something like this? Let me know if that helps.

Thanks.
Petiar.

Looks great!

As this is changing a return value for the most common DB I think we should have a change record. BC is hard here. It's perfectly reasonable for code to have done:

if ($schema->renameTable('my_table', 'my_better_named_table')) {
  // Make other changes.
}

even though this is not documented by the API someone somewhere will depend upon this behaviour - especially since this is a hangover from Drupal 7 (interestingly though not Drupal 6)

And we should only make the change in 8.7.x to give people time to adapt any code.

Needs work for the change record. The patch and tests look great.

@alexpott Not sure that return value is changing - before it was returning NULL (in worst case \Drupal\Core\Database\Connection::handleQueryException() also returns NULL) so I do not see how it may affect already existing code - I found it just typo that return used for no reason. Also test in #7 shows that core at least always getting null return code

Otoh I see \Drupal\Core\Database\Connection::defaultOptions() defines 'return' => Database::RETURN_STATEMENT as result but it is always NULL at the end, so maybe there case when it could be not null... not sure - needs other pair of eyes on it

@andypost doesn't the test in #7 prove that the return value has changed for MySQL - from a \Drupal\Core\Database\StatementInterface object to a NULL?

@alexpott the removing-return-command-2996560-7-test.patch shows that statement returned for mysql only - I guess because default options but not clear how it could be used for

Filed CR https://www.drupal.org/node/3000572

Thanks for the CR.

Committed 716828d and pushed to 8.7.x. Thanks!