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

SQLite variable limit check for migrating PR reviews #11236

Closed
1 of 7 tasks
PeterSurda opened this issue Apr 28, 2020 · 7 comments · Fixed by #11696
Closed
1 of 7 tasks

SQLite variable limit check for migrating PR reviews #11236

PeterSurda opened this issue Apr 28, 2020 · 7 comments · Fixed by #11696
Labels

Comments

@PeterSurda
Copy link

PeterSurda commented Apr 28, 2020

Description

Trying to migrate a repository from github failed when migrating pull requests is requested. I am in the process of creating a new repo to demonstrate how to reproduce the issue. I'll post an update once it's available. The error I get is
Migrate repository from xxxxxxxxx failed: too many SQL variables

I think it happens if a PR review has too many comments. SQLite has a compile time limit of arguments, defaulting to 999. I don't think the value can be increased without recompiling the library.

I tried to find the location in the code that could be causing this, and my best guess is that it's here:

if _, err := sess.NoAutoTime().Insert(review.Comments); err != nil {

As far as I can see, there is no check for the number of arguments. One row appears to have 31 columns in my case, so doing the insert in chunks of int(999/31) = 32 would fix it.

I'd try to create a PR myself but I'm not familiar with go (language, build system, tests, ...).
...

Screenshots

@PeterSurda
Copy link
Author

PeterSurda commented Apr 28, 2020

This is the review that triggers it: Bitmessage/PyBitmessage#1367 (review)

I'm trying to create a new minimal repo to reproduce it.

@guillep2k
Copy link
Member

Can you check the log for errors? Look for entries containing [E].

@lunny
Copy link
Member

lunny commented Apr 29, 2020

We should batch insert every 100 records.

@PeterSurda
Copy link
Author

PeterSurda commented Apr 29, 2020

I updated with gist of log. As you can see, the row contains 31 colums. To remain under the limit, there can be a max of 32 rows in one INSERT query of this type.

@PeterSurda
Copy link
Author

I managed to create a small repo that triggers this: https://github.com/PeterSurda/gitea-sqlite-limit

@PeterSurda
Copy link
Author

Workaround: https://gist.github.com/PeterSurda/fb2c3f8fc1b2bd1208fd691cbc302a0a

Works on both mentioned repos. It's the first code I ever wrote in golang so it may be crude.

@PeterSurda
Copy link
Author

I confirm this fixed the issue for me, using the latest docker image from docker hub.

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants