-
Notifications
You must be signed in to change notification settings - Fork 2.1k
repair search lucene before installing #10962
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
Conversation
|
🚀 Test Passed. 🚀 |
|
This will run on every update which seems not to be intended or at least may make things notably slower if everybody does it this way. I'd compare against the app version and make it only run once. |
|
The repair step only deletes duplicate rows which should not be noticable. Try estimating SELECT fileid FROM oc_lucene_status GROUP BY fileid HAVING count(fileid)>1and you will see that it is quite fast. With the primary key in place in search_lucene v0.6.0 it is even faster. |
|
And well, it is intended, because the migration might break when there are duplicates. |
|
this looks good to me but can´t test at the moment. 👍 can someone else hep with properly test this? |
lib/repair/searchlucenetables.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the correct author?
|
It's probably obvious, but anyway: This repair step is solely for the lucene application and as such this does not belong into the core repository. |
d028035 to
8e2acb1
Compare
|
A new inspection was created. |
|
@bantu all cleaned up, please see #10205 (comment) on why this has to live in core. issue to track that is in #10980 |
|
💣 Test Failed. 💣 |
|
👍 |
|
Makes sense for me 👍 |
repair search lucene before installing
|
@butonic Please backport |
|
backport to stable7 in 463ad5a |
on the way to fixing search lucene migrations, see discussion in #10205