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

Migrate sync journal table structure to support BIGINT switch on server's DB #5933

Closed
SamuAlfageme opened this issue Aug 1, 2017 · 4 comments
Assignees
Labels
p2-high Escalation, on top of current planning, release blocker Server Involved
Milestone

Comments

@SamuAlfageme
Copy link
Contributor

Once support for big fileids (owncloud/core#26901 & owncloud/core#28364) is merged in core, we'll need to review the way the client stores the fileids on the sync journal.

Also, review the impact this switch might have in the private link feature: #5763 - I remember there was some intermediate step for filling the id with zeroes in #5763 (comment), maybe we now need more padding (?) cc/ @ckamm

In owncloud/core#26901 (comment) there's a list of all affected columns on the server's DB.

@SamuAlfageme SamuAlfageme added p2-high Escalation, on top of current planning, release blocker Server Involved labels Aug 1, 2017
@SamuAlfageme SamuAlfageme added this to the 2.4.0 milestone Aug 1, 2017
@guruz
Copy link
Contributor

guruz commented Aug 3, 2017

At least three things to it:

@ckamm
Copy link
Contributor

ckamm commented Sep 5, 2017

BIGINT is roughly an 8 byte integer on MySQL, MariaDB and PostgreSQL. So our sqlite layer is already safe. We do need to go through the code and ensure we use a fitting datatype though. And yes, the conversion from fileid to id needs a double check to be in line with what the server does.

@ckamm
Copy link
Contributor

ckamm commented Sep 5, 2017

On further investigation, the client is already fully prepared for this change. We store the full id as text in the database and extract the numeric id (as text) from it.

I've commented about potential effects on the full id at owncloud/core#26901 (comment) . Once that receives an answer, I'd add a unittest and close this issue.

@ckamm
Copy link
Contributor

ckamm commented Sep 5, 2017

I've also tested with master owncloud server by manually assigning a fileid beyond the 4-byte range in the servers database. The client database looks fine and the link generation works.

@ckamm ckamm closed this as completed Sep 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-high Escalation, on top of current planning, release blocker Server Involved
Projects
None yet
Development

No branches or pull requests

3 participants