Skip to content

Enable including database views into backups #2064

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.

Sign up for GitHub

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

Jump to bottom
Merged
bryannielsen merged 3 commits into from
Oct 24, 2022
Merged

Enable including database views into backups #2064

bryannielsen merged 3 commits into from
Oct 24, 2022

Conversation

@CreepCaster
Copy link
Contributor

@CreepCaster CreepCaster commented May 4, 2022 •
edited by intoeetive
Loading

EE6 version of #2448

Overview

Fix backup for database views - previously it throws exception, because "CREATE TABLE" for "view" structure is incorrect.
https://dev.mysql.com/doc/refman/8.0/en/show-table-status.html

For views, most columns displayed by SHOW TABLE STATUS are 0 or NULL except that Name indicates the view name, Create_time indicates the creation time, and Comment says VIEW

Nature of This Change

  • 🐛 Fixes a bug
  • 🚀 Implements a new feature
  • 🛁 Refactors existing code
  • 💅 Fixes coding style
  • ✅ Adds tests
  • 👽 Adds new dependency
  • 🔥 Removes unused files / code
  • 🔒 Improves security

Is this backwards compatible?

  • Yes
  • No

@CLAassistant
Copy link

CLAassistant commented May 4, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@intoeetive intoeetive added this to the 6.4.0 milestone Jul 28, 2022
intoeetive
intoeetive requested changes Aug 16, 2022
View reviewed changes
Copy link
Contributor

@intoeetive intoeetive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @CreepCaster ,

thank you for your contribution - and sorry for late reaction.

I think the code is good addition - even though EE does not use MySQL views internally, the add-ons or custom site setups can use those.
However, it looks like the change from $this->getTables(); to $this->query->getTables(); is making setTablesToBackup() function impossible to use (and thus making CMS-specific backups not possible)

Is that something you'd be willing to look into more?

@CreepCaster
Copy link
Contributor Author

CreepCaster commented Aug 27, 2022

Hi @intoeetive ,
thank you for review. Really my bad, fixed.

@CreepCaster CreepCaster requested a review from intoeetive August 31, 2022 08:57
@intoeetive
Copy link
Contributor

intoeetive commented Sep 6, 2022

Reviewed the code and tested - looks good and works well. Approving.

@CreepCaster would you mind creating another PR from same branch into 7.dev (for ExpressionEngine 7)?

@intoeetive intoeetive changed the title backup database views Enable including database views into backups Oct 21, 2022
@intoeetive intoeetive mentioned this pull request Oct 21, 2022
Merged
@bryannielsen bryannielsen merged commit a7e4f53 into ExpressionEngine:6.dev Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bryannielsen bryannielsen bryannielsen approved these changes

@intoeetive intoeetive intoeetive approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

6.4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@CreepCaster @CLAassistant @intoeetive @bryannielsen