search

etianen / django-watson

Full-text multi-table search application for Django. Easy to install and use, with good performance.
BSD 3-Clause "New" or "Revised" License
1.2k stars 129 forks source link

add missing index on SearchEntry.object_id #301

Closed syphar closed 1 year ago

syphar commented 1 year ago

Debugging some performance issues with django-watson I stumbled onto this issue.

For objects with a non-integer primary key the text-object-id is used (among other places) for updates of the search-index.

Without index this can be slow: in our case updating a single row takes 12 seconds (2.5 Mio records in the table). With the index added it's much much faster.

etianen commented 1 year ago

Excellent, thank you! The build issues look unrelated. I'll fix them and push out a release today.

etianen commented 1 year ago

Unfortunately, I've had to back this out, as it caused issues with MySQL. See:

https://github.com/etianen/django-watson/actions/runs/3862668749/jobs/6584383141

I don't have a local MySQL setup to replicate this at the moment. If you can submit a new PR that passes the build checks, I'd be very happy to merge!

syphar commented 1 year ago

I'll have a look on monday, seems odd that mysql doesn't support db_index on a normal TextField.

Worst case we probably have to migrate this field to a models.CharField with a certain length, which is a fine and quick migration for postgres, I'm not sure about mysql.

What are your thoughts about this?

etianen commented 1 year ago

Migrating to a char field with a max length would work. I didn't spot it was a text field rather than a char field! It would need to be a limit of 191 to work with all versions of MySQL. I can't remember why, but I've got this working before in another project (django-reversion).

On Sat, 7 Jan 2023 at 16:45, Denis Cornehl @.***> wrote:

I'll have a look on monday, seems odd that mysql doesn't support db_index on a normal TextField.

Worst case we probably have to migrate this field to a models.CharField with a certain length, which is a fine and quick migration for postgres, I'm not sure about mysql.

What are your thoughts about this?

— Reply to this email directly, view it on GitHub https://github.com/etianen/django-watson/pull/301#issuecomment-1374537349, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEKCDJPOUFBMFCYEAF4YDWRGMRHANCNFSM6AAAAAATR4XEDY . You are receiving this because you modified the open/close state.Message ID: @.***>

syphar commented 1 year ago

I realized only just now that django-watson and django-reversion are both by you :D Thank you for both!

etianen commented 1 year ago

Yup! And both are a little neglected these days, what with a busy full-time job and a busy full-time life! Glad you like them!