Skip to content

Update GetDatabaseConfig.php #113

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

Merged
merged 1 commit into from
Dec 30, 2018
Merged

Conversation

robbielove
Copy link
Contributor

Please consider adding a way to include extra params for mysqldump, this is possible in backup-manager but not laravel:
// add additional options to dump-command (like '--max-allowed-packet')
'extraParams' => '--column-statistics=0',

My pull request has '--column-statistics=0' hard coded for reasons detailed below, however I suggest that this should not be hard coded - provide a way for these values to be specified, possibly with another config file/array; 'extraParams' => $connection['extraParams'],?

add compatibility for mysql 8.0 where column-statistics is not present anymore and will cause backups to fail, this change is needed because backup manager allows extra params and it is possible to set --column-statistics=0, however backup-manager/laravel just passes the config from backup-manager - providing no opportunity for these extra params to be included

add compatibility for mysql 8.0 where column-statistics is not present anymore and will cause backups to fail, this change is needed because backup manager allows extra params and it is possible to set `--column-statistics=0`, however backup-manager/laravel just passes the config from backup-manager - providing no opportunity for these extra params to be included
@ShawnMcCool ShawnMcCool merged commit 27bbe5d into backup-manager:master Dec 30, 2018
@ShawnMcCool
Copy link

Thanks for this. =)

@nafiesl nafiesl mentioned this pull request Oct 5, 2019
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.

2 participants