Skip to content
This repository has been archived by the owner on Feb 20, 2019. It is now read-only.

Add details about configuring 4-byte Unicode Support in MySQL #2926

Merged
merged 5 commits into from Apr 27, 2017
Merged

Add details about configuring 4-byte Unicode Support in MySQL #2926

merged 5 commits into from Apr 27, 2017

Conversation

settermjd
Copy link
Contributor

References

#2924

@settermjd settermjd self-assigned this Mar 9, 2017
@mention-bot
Copy link

@settermjd, thanks for your PR! By analyzing the history of the files in this pull request, we identified @RandolfCarter, @butonic and @wshadow to be potential reviewers.

@PVince81
Copy link
Contributor

PVince81 commented Mar 9, 2017

@DeepDiver1975 please review the doc

@mmattel
Copy link
Contributor

mmattel commented Mar 9, 2017

Question:
... When this is done, tables in MySQL will be created ...

Reading this, I expect that NEW tables will be created as written.

When you have a running ownCloud instance, how to enable it there ?

@settermjd
Copy link
Contributor Author

@mmattel in answer to your question, I took that sentence from the following snippet from config/config.php:

Tables will be created with
  * character set: utf8mb4
  * collation:     utf8mb4_bin
  * row_format:    compressed

DeepDiver1975
DeepDiver1975 suggested changes Mar 9, 2017
View reviewed changes
admin_manual/configuration_database/linux_database_configuration.rst Outdated
For supporting such features as emoji, you have to both enable 4-byte Unicode support in MySQL (instead of the default 3) as well as in ownCloud.
To do so:

1. In ``config/config.sample.php`` set ``mysql.utf8mb4`` to true
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not necessary - during setup we are detecting if mb4 is possible .... I'm sorry for not providing the correct information.
Let me double check config.sample.php - maybe this needs some adoptions as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @DeepDiver1975. Hear from you when you're ready.

Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid confusion I would explicitly state this is for new installations only.
Q: is there a migration path available?
Q: what about DB backup and restore - any issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DeepDiver1975 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@settermjd pls see also: owncloud/core#27409 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mmattel, I'm just now coming back to this and as I'm reading through the discussion here, the code, and owncloud/core#27409 (comment) the point about it being for new installations, not upgrades seems incorrect. Can you clarify?

@mmattel mmattel mentioned this pull request Mar 17, 2017
Closed
@mmattel
Copy link
Contributor

mmattel commented Mar 28, 2017

@settermjd changes have been made to config.sample.php to describe the behaviour briefly.
You could now incooperate those here. Then this PR would be fine

@PVince81
Copy link
Contributor

Any update ? @guruz

@mmattel
Copy link
Contributor

mmattel commented Apr 26, 2017

No refrence to config.sample.php?
This would imho help...

@settermjd
Copy link
Contributor Author

Should have that in in tonight's session.

@guruz
Copy link
Contributor

guruz commented Apr 26, 2017 •
edited 

 -...  All instructions work for both. You will not need to replace mysql with anything.
 +...  All instructions work for both. You will not need to replace MySQL with anything.

Here mysql is about the command on the linux shell. Not MySQL the product name. So it should stay lowercase. The shell command does not need to be replaced(?)

@settermjd
Copy link
Contributor Author

@guruz can you review ec5cf6a? I hope that we understand each other fully.

guruz
guruz reviewed Apr 26, 2017
View reviewed changes
admin_manual/configuration_database/linux_database_configuration.rst Outdated
@@ -83,7 +83,7 @@ If you decide to use a MySQL or MariaDB database, ensure the following:
* That you have installed and enabled the ``pdo_mysql`` extension in PHP
* That the **mysql.default_socket** points to the correct socket (if the database runs on the same server as ownCloud).

.. note:: MariaDB is backwards compatible with MySQL. All instructions work for both. You will not need to replace MySQL with anything.
.. note:: MariaDB is backwards compatible with MySQL. All instructions work for both, so you will not need to replace or revise any, existing, MySQL client commands.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@settermjd
Copy link
Contributor Author

@guruz @PVince81 @mmattel - better/ready?

@PVince81 PVince81 mentioned this pull request Apr 26, 2017
Closed
3 tasks
@PVince81
Copy link
Contributor

leaving this up to you guys @guruz @mmattel

@settermjd
Copy link
Contributor Author

@guruz / @mmattel if you're happy with it, either give me a thumbs up or just merge & delete pls.

@mmattel
Copy link
Contributor

mmattel commented Apr 27, 2017

👍

@settermjd settermjd merged commit 7e0eea8 into master Apr 27, 2017
@settermjd settermjd deleted the document-configure-mysql-for-4-byte-unicode branch April 27, 2017 13:28
@guruz
Copy link
Contributor

guruz commented May 3, 2017

https://www.youtube.com/watch?v=71hqRT9U0wg

@settermjd
Copy link
Contributor Author

@guruz - my little girl's been singing that with her Grandma for a few weeks now. Great timing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@guruz guruz guruz left review comments

@mmattel mmattel mmattel left review comments

@DeepDiver1975 DeepDiver1975 DeepDiver1975 requested changes

@PVince81 PVince81 Awaiting requested review from PVince81

Assignees

@settermjd settermjd

Labels
Admin WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@settermjd @mention-bot @PVince81 @mmattel @guruz @DeepDiver1975