Skip to content

Comments

Enable including database views into backups#2064

Merged
bryannielsen merged 3 commits intoExpressionEngine:6.devfrom
CreepCaster:bug/backup-for-database-views
Oct 24, 2022
Merged

Enable including database views into backups#2064
bryannielsen merged 3 commits intoExpressionEngine:6.devfrom
CreepCaster:bug/backup-for-database-views

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