Missing columns after OC 10.5 -> NC 21.0.0beta2 upgrade

[PVince81]

Steps to reproduce

1. Upgrade from OC 10.5.0 to NC 21.0.0beta2 by replacing the source code and keeping config and data
2. Run occ upgrade => no errors
3. Run db:convert-filecache-bigint

Expected behaviour

Success

Actual behaviour

Error about missing columns "oc_share_external.parent"

Missing columns

Expected from NC 21.0.0beta2

MariaDB [nextcloud_beta]> describe oc_share_external;
+-----------------+---------------+------+-----+---------+----------------+
| Field           | Type          | Null | Key | Default | Extra          |
+-----------------+---------------+------+-----+---------+----------------+
| id              | bigint(20)    | NO   | PRI | NULL    | auto_increment |
| parent          | bigint(20)    | YES  |     | -1      |                |
| share_type      | int(11)       | YES  |     | NULL    |                |
| remote          | varchar(512)  | NO   |     | NULL    |                |
| remote_id       | varchar(255)  | YES  |     |         |                |
| share_token     | varchar(64)   | NO   |     | NULL    |                |
| password        | varchar(64)   | YES  |     | NULL    |                |
| name            | varchar(64)   | NO   |     | NULL    |                |
| owner           | varchar(64)   | NO   |     | NULL    |                |
| user            | varchar(64)   | NO   | MUL | NULL    |                |
| mountpoint      | varchar(4000) | NO   |     | NULL    |                |
| mountpoint_hash | varchar(32)   | NO   |     | NULL    |                |
| accepted        | int(11)       | NO   |     | 0       |                |
+-----------------+---------------+------+-----+---------+----------------+

Actual

MariaDB [owncloud]> describe oc_share_external;
+-----------------+---------------------+------+-----+---------+----------------+
| Field           | Type                | Null | Key | Default | Extra          |
+-----------------+---------------------+------+-----+---------+----------------+
| id              | bigint(20)          | NO   | PRI | NULL    | auto_increment |
| remote          | varchar(512)        | NO   |     | NULL    |                |
| share_token     | varchar(64)         | NO   |     | NULL    |                |
| password        | varchar(64)         | YES  |     | NULL    |                |
| name            | varchar(64)         | NO   |     | NULL    |                |
| owner           | varchar(64)         | NO   |     | NULL    |                |
| user            | varchar(64)         | NO   | MUL | NULL    |                |
| mountpoint      | varchar(4000)       | NO   |     | NULL    |                |
| mountpoint_hash | varchar(32)         | NO   |     | NULL    |                |
| remote_id       | varchar(255)        | NO   |     | -1      |                |
| accepted        | int(11)             | NO   |     | 0       |                |
| lastscan        | bigint(20) unsigned | YES  |     | NULL    |                |
+-----------------+---------------------+------+-----+---------+----------------+

=> need to:

• add "parent" column
• remove "lastscan" column
• PR for share_external here: Add migration for oc_share_external columns #24829

@juliushaertl not sure how these were missed, maybe some of the migrations did not run

[PVince81]

• oc_systemtag.assignable column not properly deleted: https://github.com/nextcloud/server/pull/23044/files#r547282236

  • also need to delete it from the key

• oc_systemtag_object_mapping has unique key instead of PRIMARY KEY (objecttype,objectid,systemtagid) => solved by occ command

[PVince81]

• oc_comments.reference_id is missing => optional
• oc_comments_read_markers index differences solved by occ command
• oc_dav_properties not deleted?

[PVince81]

• oc_federated_reshares:
  • remote_id default is different
  • index is not primary key solved by occ command

is:

  `remote_id` varchar(255) COLLATE utf8mb4_bin NOT NULL COMMENT 'share ID at the remote server',

should be:

  `remote_id` varchar(255) COLLATE utf8mb4_bin DEFAULT '',

[nickvergessen]

oc_comments.reference_id is missing

That's an optional column which can be added with an occ command

[PVince81]

• oc_filecache missing fs_mtime and fs_size keys => solved by occ command

[PVince81]

• oc_notifications, missing icon column: but this is because I had the notifications app disabled for many many OC versions and now when enabling the NC notifications app, the column did not get added. Not sure if / how we want to support such cases...

[PVince81]

• should we delete oc_privatedata ?

• oc_properties differences:

  • field lengths
  • defaults
  • missing "property_index": should probably be added to the occ command ?

is:

CREATE TABLE `oc_properties` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `userid` varchar(64) COLLATE utf8mb4_bin DEFAULT '',
  `propertypath` varchar(255) COLLATE utf8mb4_bin NOT NULL,
  `propertyname` varchar(255) COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `propertyvalue` varchar(255) COLLATE utf8mb4_bin NOT NULL,
  PRIMARY KEY (`id`),
  KEY `properties_path_index` (`userid`,`propertypath`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=COMPRESSED;

should be:

CREATE TABLE `oc_properties` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `userid` varchar(64) COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `propertypath` varchar(255) COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `propertyname` varchar(255) COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `propertyvalue` longtext COLLATE utf8mb4_bin NOT NULL,
  PRIMARY KEY (`id`),
  KEY `property_index` (`userid`),
  KEY `properties_path_index` (`userid`,`propertypath`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=COMPRESSED;

[PVince81]

I could not run the index occ yet due to the bigint one blocking, so maybe some of the index discrepancies will be solved there.

[PVince81]

I have a local solution for the first bits (share_external and assignable)

[PVince81]

dropping assignable is more tricky as it will require adjusting the indices, so I abandoned that one for now and focussed on share_external.

[PVince81]

after patching my server with #24829 the occ commands all ran fine!

additionally to the list from #23044 (comment) the setup checks told me to run "occ db:add-missing-primary-keys" which worked as well.

[PVince81]

I redid a diff and went through the items on top, most of the index differences were solved by the occ command.

Here a few more I found:

• oc_activity.app length different (was known before, probably still pending)
• oc_activity indices are different:
  ist:

  PRIMARY KEY (`activity_id`),
  KEY `activity_user_time` (`affecteduser`,`timestamp`),
  KEY `activity_filter_by` (`affecteduser`,`user`,`timestamp`),
  KEY `activity_filter_app` (`affecteduser`,`app`,`timestamp`),
  KEY `activity_object` (`object_type`,`object_id`)

soll:

  PRIMARY KEY (`activity_id`),
  KEY `activity_user_time` (`affecteduser`,`timestamp`),
  KEY `activity_filter_by` (`affecteduser`,`user`,`timestamp`),
  KEY `activity_filter` (`affecteduser`,`type`,`app`,`timestamp`),
  KEY `activity_object` (`object_type`,`object_id`)

• oc_activity_mq index name: is=authtoken_last_activity_index but must be authtoken_last_activity_idx (in case of future reference in migrations)

• oc_external_config.value is NOT NULL but must be DEFAULT NULL

• oc_filestrash.auto_id is bigint(20) => must also be adjusted in future NC versions, possibly through the occ command: Make oc_files_trash.auto_id a bigint #24832

• oc_share missing has extra indices "item_source_index" and "item_source_type_index" => add to NC as well ?

• oc_share_external.remote_id: is not null must be default ''

[nickvergessen]

oc_systemtag.assignable column not properly deleted

#25001

[PVince81]

#25001 likely broke the upgrade.

we need to adjust the key before deleting the column:

Here's the diff:
OC:

  UNIQUE KEY `tag_ident` (`name`,`visibility`,`editable`,`assignable`)

NC:

  UNIQUE KEY `tag_ident` (`name`,`visibility`,`editable`)

So we need to delete the "tag_ident" key and then recreate it without the "assignable" column.
And only then we can delete the column.

[PVince81]

Should the key be adjusted directly as part of the migration or be part of the occ command ? The latter would be tricky as we'd need to delete the column afterwards in a subsequent update/migration.

[PVince81]

Also unclear: what to do if the "assignable" had a value ? Some tags might become duplicate so we need to make sure that there is no error when "merging" the tags during an upgrade. Tricky stuff :-/

[PVince81]

I've pushed a PR for updating tag_ident before removing the column: #25068

[PVince81]

I grepped for "assignable" and saw that we do have code that works with it, so might need to delete that as well: https://github.com/nextcloud/server/blob/master/apps/systemtags/lib/Activity/Provider.php#L333

In any case, it seems the "assignable" column needs further careful consideration and handling.

For now to avoid blocking NC 20, this PR removes assignable column removal during upgrade, for now. We can handle it in a later minor release: #25069

[szaimen]

Hi, please update to 24.0.9 or better 25.0.3 and report back if it fixes the issue. Thank you!

My goal is to add a label like e.g. 25-feedback to this ticket of an up-to-date major Nextcloud version where the bug could be reproduced. However this is not going to work without your help. So thanks for all your effort!

If you don't manage to reproduce the issue in time and the issue gets closed but you can reproduce the issue afterwards, feel free to create a new bug report with up-to-date information by following this link: https://github.com/nextcloud/server/issues/new?assignees=&labels=bug%2C0.+Needs+triage&template=BUG_REPORT.yml&title=%5BBug%5D%3A+

[PVince81]

@szaimen this isn't really an issue / bug per se, but a diff of the DB schema from OC to NC to help migration

so far I haven't heard of any critical issues there due to missing indices or so in the past years, so the ticket itself is not critical

we can either convert to "tech debt" or close

[szaimen]

Thanks! Lets close then