Skip to content
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

occ db:convert-type doesn't work with migration #27075

Open
PVince81 opened this issue Feb 2, 2017 · 61 comments
Open

occ db:convert-type doesn't work with migration #27075

PVince81 opened this issue Feb 2, 2017 · 61 comments

Comments

@PVince81
Copy link
Contributor

PVince81 commented Feb 2, 2017

Steps

  1. Setup ownCloud with sqlite
  2. Use occ db:convert-type to convert to MySQL
  3. Refresh the page

Expected

All tables migrated

Actual

Some tables were omitted: oc_external_config, etc. These tables are created not with MDB2 schemas but with migrations.

Versions

master.

We need to adjust the command to also run the migrations on the new database to initially create the tables. I'm not sure if this will work correctly because a migration isn't always only creating tables...

± % occ db:convert-type --all-apps --password owncloud42 mysql owncloud localhost owncloudmb4
Creating schema in new database
The following tables will not be converted:
oc_external_applicable
oc_external_config
oc_external_mounts
oc_external_options
oc_migrations
Continue with the conversion (y/n)? [n] y
oc_activity
    0 [>---------------------------]oc_activity_mq
    0 [>---------------------------]oc_addressbookchanges
    0 [>---------------------------]oc_addressbooks
 1/1 [============================] 100%oc_appconfig
 66/66 [============================] 100%oc_authtoken
 1/1 [============================] 100%oc_calendarchanges
    0 [>---------------------------]oc_calendarobjects
    0 [>---------------------------]oc_calendars
 1/1 [============================] 100%oc_calendarsubscriptions
    0 [>---------------------------]oc_cards
    0 [>---------------------------]oc_cards_properties
    0 [>---------------------------]oc_comments
    0 [>---------------------------]oc_comments_read_markers
    0 [>---------------------------]oc_credentials
    0 [>---------------------------]oc_dav_shares
    0 [>---------------------------]oc_federated_reshares
    0 [>---------------------------]oc_file_locks
 3/3 [============================] 100%oc_filecache
 14/14 [============================] 100%oc_files_trash
    0 [>---------------------------]oc_group_admin
    0 [>---------------------------]oc_group_user
 1/1 [============================] 100%oc_groups
 1/1 [============================] 100%oc_jobs
 14/14 [============================] 100%oc_mimetypes
 8/8 [============================] 100%oc_mounts
 1/1 [============================] 100%oc_preferences
 2/2 [============================] 100%oc_privatedata
    0 [>---------------------------]oc_properties
    0 [>---------------------------]oc_schedulingobjects
    0 [>---------------------------]oc_share
    0 [>---------------------------]oc_share_external
    0 [>---------------------------]oc_storages
 2/2 [============================] 100%oc_systemtag
    0 [>---------------------------]oc_systemtag_group
    0 [>---------------------------]oc_systemtag_object_mapping
    0 [>---------------------------]oc_trusted_servers
    0 [>---------------------------]oc_users
 1/1 [============================] 100%oc_vcategory
    0 [>---------------------------]oc_vcategory_to_object
    0 [>---------------------------]

@DeepDiver1975

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 2, 2017

I'm afraid that there will be no solution for this.
Migrations cannot provide a list of tables to migrate

Maybe convert-type needs to work differently: instead of using the schemas, just migrate whatever data is there regardless... in which case the command itself becomes generic and could be written as a completely separate tool (not part of ownCloud).

I'm already hearing @DeepDiver1975 from far away saying "kill convert-type" 😉

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 2, 2017

Wait a minute... The option --all-apps is supposed to also migrate extra tables.

But the code seems to just through away the extra tables instead of doing so.
When the option is not set, a hint tells that you can set it.

But then it doesn't do anything. WTF

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 2, 2017

Ok, found the code.

Actually it finds the extra tables only for the warnings.
When you specify --all-apps it uses getAllApps() instead of getEnabledApps() and reads database.xml to find out how to create the tables...

So no luck there.

@DeepDiver1975
Copy link
Member

I'm already hearing @DeepDiver1975 from far away saying "kill convert-type" 😉

😬

@DeepDiver1975
Copy link
Member

We could clear the migrations table on the target schema and run all migration steps for all apps.

somthing like:

./occ migrations:migrate --all --restart

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 3, 2017

You don't even need to clear the migration table, the convert script doesn't copy it over.

My worry about running the migration scripts is that some might want to operate on existing data. But the data hasn't been copied over yet. On the other hand, the data in the source tables is already converted to whatever format is required by the migrations once they all ran. So it might work.

I guess there can still be some possible migration scenarios where this wouldn't work, if for example a migration expects data to exist and fails if it doesn't (ex: some kind of orphaned share situation where the missing data wasn't copied yet to the target tables)

@PVince81
Copy link
Contributor Author

We need to sort this out before final. So far I don't see any good solution apart from deprecating the tool... Or extracting it to be a general DB conversion tool not part of OC releases.

@PVince81
Copy link
Contributor Author

Assigning to @DeepDiver1975 to make a decision

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 3, 2017

We'll need a decision here, this feature will be completely broken in 10.0 and I don't see any possible solution because DB migrations work completely differently than what this feature here expects.

I'd suggest removing this feature. Maybe a separate generic tool could be provided to convert a SQLite DB to MySQL but nothing bundled or OC specific.

@pmaier1

@felixboehm
Copy link
Contributor

Let the command only say:
"This function is not working in ownCloud 10.0. Please contact your ownCloud Support for consulting."
Then exit.
We can add the functionality / extra tool later, when we have a good solution.

@PVince81
Copy link
Contributor Author

not adding this now -> 10.0.1

I'm not even sure we can even make it work correctly without a huge effort, and I have the feeling that we don't have time to spend that effort for this little benefit.

@PVince81 PVince81 modified the milestones: 10.0.1, 10.0 Apr 19, 2017
@PVince81
Copy link
Contributor Author

PVince81 commented May 2, 2017

@pmaier1 how important is this feature ? I don't think it is fixable at all currently without major rework.

Should we remove it and/or add a message that it doesn't work ?

@PVince81
Copy link
Contributor Author

moving to triage for decision and scheduling

@PVince81 PVince81 modified the milestones: triage, 10.0.1 May 17, 2017
@PVince81
Copy link
Contributor Author

kill?

@huttarl
Copy link

huttarl commented Mar 8, 2018

@ghost wrote,

you should already get a warning to not use SQLite during the installation:

SQLite will be used as database.
For larger installations we recommend to choose a different database backend.
Especially when using the desktop client for file syncing the use of SQLite is discouraged.

I've installed owncloud 3 times today, and it took me this long to figure out how to follow the above advice. It turns out there is a "Storage & database" dropdown on that page, small and transparent on a detailed background, sandwiched between large, bright, opaque input objects like the login credentials fields and the "Finish installation" button. If you notice that little dropdown and click it, you get access to the ability to change database settings. If instead you first enter your credentials and press "Enter" to log in, as the primary elements at the top of the page suggest, you miss the chance to configure database settings.

The warning that SQLite is discouraged, with documentation saying that you can migrate to something else once you get the service up and running, is very different from a warning that says use of SQLite is discouraged and it will be difficult to change later.

When I was installing oc, and encountered the above message, I searched the documentation for how to choose MySQL instead of SQLite. All I found was advice about running occ to convert the database to MySQL after installation. So I went with that option. Of course that led down a frustrating trail.

If you decide not to fix the conversion-to-MySQL tool, please make the Storage and Database settings on the installation page much more prominent. Make them visible instead of hidden by default. And maybe even ask for confirmation if the user tries to click the Finish Installation button with SQLite still selected, letting them know that there is not a conversion path from SQLite.

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 8, 2018

or better: drop sqlite support altogether #29625 (comment)

@PVince81
Copy link
Contributor Author

need time from QA at some point to test this when time permits @patrickjahns

@rienex
Copy link

rienex commented Aug 7, 2018

is there any news about the bug? Is there any workaround?

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 8, 2018

you can help testing whether the PR works for you: #30643

@TByte007
Copy link

After 2 years and no fix and now market app stopped working with SQLite.

@pmaier1
Copy link
Contributor

pmaier1 commented Aug 20, 2019

THX @VicDeo !

The above mentioned fix will be part of oC Server 10.3. Please try it and provide feedback here!

@jacobklitzke
Copy link

Thanks @pmaier1 ! I have tested this on OwnCloud version 10.3.0 RC1. On migration from SQlite to MariaDB using the following syntax:

sudo -u apache php occ db:convert-type mysql owncloud 127.0.0.1 owncloud

I receive the following errors:

This feature is currently experimental.
Enter a password to access a target database: 
Creating schema in new database

In AbstractMySQLDriver.php line 115:                                                                                                     
  An exception occurred while executing 'ALTER TABLE oc_calendarsubscriptions CHANGE `lastmodified` lastmodified INT UNSIGNED DEFAULT NULL NOT NULL':                                                                                                                      
  SQLSTATE[42000]: Syntax error or access violation: 1067 Invalid default value for 'lastmodified'                                                     
                                                                                                                                                       
In PDOConnection.php line 106:                                                  
  SQLSTATE[42000]: Syntax error or access violation: 1067 Invalid default value for 'lastmodified'  
                                                                                                    
In PDOConnection.php line 104:                                                         
  SQLSTATE[42000]: Syntax error or access violation: 1067 Invalid default value for 'lastmodified'  

@VicDeo
Copy link
Member

VicDeo commented Oct 7, 2019

@jacobklitzke sounds similar to #29445
What is your MariaDB version?

@VicDeo
Copy link
Member

VicDeo commented Oct 8, 2019

ok. got it.
Until PHP7.0 support is dropped we use a column listener workaround to fix migrations produced by doctrine/dbal 2.5 for MariaDB 10.2.7+

The workaround is activated by Platform name and version. When migrations are generated the platform is not MariaDB (source DB is checked in lines below while MariaDB is a target DB )

https://github.com/owncloud/core/blob/master/lib/private/DB/MySqlSchemaColumnDefinitionListener.php#L51
https://github.com/owncloud/core/blob/master/lib/private/DB/MySqlSchemaColumnDefinitionListener.php#L54

@VicDeo
Copy link
Member

VicDeo commented Oct 8, 2019

@jacobklitzke
For MariaDB 10.2.7+ please try changing https://github.com/owncloud/core/blob/master/lib/private/DB/MySqlSchemaColumnDefinitionListener.php#L58
to
if (true) {
it will activate the workaround unconditionally.

I don't think it worth fixing as 10.4 will probably unlock doctrine/dbal 2.7 by dropping PHP 7.0.
doctrine/dbal 2.7 has native MariaDB 10.2.7+ support.

@jacobklitzke
Copy link

@VicDeo After making the change you recommended, the migration worked successfully. Thanks for all your help and work on this issue! BTW, I was using MariaDB 10.4.7.

settermjd added a commit to owncloud/docs that referenced this issue Jan 27, 2020
This change is being made because the database conversion command is
currently broken. When owncloud/core#27075 is
merged, this command can be re-enabled. This fixes #2254.
@KlausThornProgrammfabrik

Worked for me: sqlite to mysql conversion with the official docs:

https://doc.owncloud.com/server/10.5/admin_manual/configuration/database/db_conversion.html
(but table_cache is now table_open_cache)

owncloud server 10.5.0.10

mysql-server 5.7.31-0ubuntu0.18.04.1

PHP 7.2.24-0ubuntu0.18.04.6 (but php 7.0 is also installed. No clue which one is used)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.