This part is pretty much useless when elastic is being used for search.
- Mentioned In
- T219303: [Checkpoint 5] Update Read Logic
T210618: Wikidata entity search sometimes case-sensitive, uses wb_terms instead of CirrusSearch
T181486: Missing terms in wb_terms table
T189779: Run clearTermSqlIndexSearchFields on Wikidata
T189777: Disable reading from term_search_key from wb_terms table in wikidata
T189776: Disable reading from term_search_key from wb_terms table in beta cluster and testwikidata
T188992: Make wb_terms table fancy
Currently, term_search_key is still used for TermSqlIndex’ implementation of the LabelConflictFinder interface – to find entities with the same label and description (you’re not allowed to produce such conflicts when editing or creating entities). Before we can wipe the column, we need to decide what to do with this – we can either add another LabelConflictFinder implementation based on Cirrus/Elastic, or use term_text instead of term_search_key in conflict detection (and accept that “foo” and “foO” will no longer be detected as a conflict).
- write empty values if search uses Cirrus/Elastic
- add maintenance script to wipe the columns (in batches, etc.) – not yet sure if this should be part of update.php
- add maintenance script to re-populate the columns and note in docs/options.wiki that admins who disable Cirrus/Elastic again must make sure to run this script
Before we can wipe the column, we need to decide what to do with this – we can either add another LabelConflictFinder implementation based on Cirrus/Elastic, or use term_text instead of term_search_key in conflict detection (and accept that “foo” and “foO” will no longer be detected as a conflict).
Some more information – using term_text doesn’t only mean case sensitivity in the search, it also means that «thé» and «thé» will be considered different (one uses a precomposed character, one a combining diacritic), or “foo” and “foo” (the second one starts with an LTR mark), or “foo” and “foo ”. The term_search_key normalization takes care of all that for us (Unicode normalization, removal of control characters, stripping of leading and trailing whitespace, and finally case conversion).
@Lydia_Pintscher any opinion on this? Do you think it’s acceptable to detect conflicts based only on the pure, non-normalized term text?
Yet more information :) @Ladsgroup pointed out that some of these normalizations are also performed by Wikibase on any term (not just for the search key), specifically Unicode normalization and whitespace stripping. However, removal of control characters doesn’t seem to be part of that – I can add the same alias twice, once with a leading LTR character, and the database will have two rows with the same term_search_key but different term_texts. But I guess that’s still acceptable.
Okay, that’s because StringNormalizer only removes category Cc (other, control) whereas TermSqlIndex also removes Cf (other, format), Cn (other, not assigned) and Cs (other, surrogate). I doubt that’s intentional… perhaps we should clean that up anyways.
Yeah, at least ZWNJ, ZWJ and ZWSP are in Cf and widely used. Also, due to our outdated PHP version some recently added codepoints for regular characters are misclassified as Cn (unassigned), we shouldn’t remove those either.
It’s probably acceptable to use the less normalized term_text for conflict search, though, so I think we can leave the normalization situation as it is for now.
TermPropertyLabelResolver also uses a TermIndex, so if I understand correctly that means there’s a small chance that a Lua module referring to a property by its non-normalized label will stop working…
I’d feel a lot more comfortable with this change if we split it into two phases – first stop using the columns, then stop writing them. We don’t have to maintain this separation forever, but for the initial deployment I’d like to be able to roll back this change without having to worry about database rows where the search key was removed while the change was in effect.
Also, I have no idea what to do in the client, where we also use TermSqlIndex but don’t have a useCirrus setting…
Perhaps this should be a separate setting and not automatically inferred from useCirrus, so that we can have it in both repo and client?
Still to be done:
- enable those config settings on wikidata (and testwikidata, beta, etc.)
- if everything goes okay, disable the “force writing” setting again
- run maintenance script to kill term_search_key and term_weight. I think rebuildTermSqlIndex.php might do the job, but that should be tested manually. rebuildTermsSearchKey always writes the full search key afaict, so that’s not what we want (perhaps it should be updated)
but those should probably be separate tasks.