Skip to content

Commit a75026f

Browse files
committed
#546: Keep name lookups index-friendly on MySQL and MariaDB
existsPage(String) and Category(Wikipedia, String) appended COLLATE utf8mb4_bin to their only name comparison, which keeps MySQL from using the name index and turns every lookup into a full scan. The queries now compare in the collation of the column and the exact, case-sensitive match is picked in Java from the few case variants returned. A dual predicate (plain plus collated comparison) was not used: MySQL 8.4 drops the collated one as redundant with the index lookup, so case variants matched, and a column-side COLLATE fails on utf8mb3 columns.
1 parent 179133b commit a75026f

5 files changed

Lines changed: 154 additions & 28 deletions

File tree

‎dkpro-jwpl-api/src/main/java/org/dkpro/jwpl/api/Category.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.dkpro.jwpl.api;
1919

2020
import java.util.HashSet;
21+
import java.util.List;
2122
import java.util.Set;
2223

2324
import org.dkpro.jwpl.api.exception.WikiApiException;
@@ -40,6 +41,16 @@ public class Category
4041
implements WikiConstants
4142
{
4243

44+
/**
45+
* The native query {@link #createCategory(Title)} runs to find a category, taking its name as
46+
* parameter {@code name}. It yields the category entities found. The comparison is done in the
47+
* collation of the column, which keeps the index on the column usable, so the caller has to
48+
* pick the exact match.
49+
*
50+
* @see Wikipedia#PAGE_NAMES_BY_NAME_QUERY
51+
*/
52+
static final String CATEGORY_BY_NAME_QUERY = "select * from Category where name = :name";
53+
4354
private final CategoryDAO catDAO;
4455
private org.dkpro.jwpl.api.hibernate.Category hibernateCategory;
4556
private final Wikipedia wiki;
@@ -134,12 +145,19 @@ private void createCategory(Title title) throws WikiPageNotFoundException
134145
{
135146
String name = title.getWikiStyleTitle();
136147

137-
final String query = "select * from Category where name = :name"
138-
+ (wiki.getDatabaseConfiguration().supportsCollation() ? Wikipedia.SQL_COLLATION
139-
: "");
140-
hibernateCategory = wiki.__inTransaction(session -> session
141-
.createNativeQuery(query, org.dkpro.jwpl.api.hibernate.Category.class)
142-
.setParameter("name", name, String.class).uniqueResult());
148+
List<org.dkpro.jwpl.api.hibernate.Category> candidates = wiki.__inTransaction(
149+
session -> session
150+
.createNativeQuery(CATEGORY_BY_NAME_QUERY,
151+
org.dkpro.jwpl.api.hibernate.Category.class)
152+
.setParameter("name", name, String.class).list());
153+
154+
// the query compares in the collation of the column, so keep the exact match only
155+
for (org.dkpro.jwpl.api.hibernate.Category candidate : candidates) {
156+
if (name.equals(candidate.getName())) {
157+
hibernateCategory = candidate;
158+
break;
159+
}
160+
}
143161

144162
// if there is no category with this name, the hibernateCategory is null
145163
if (hibernateCategory == null) {

‎dkpro-jwpl-api/src/main/java/org/dkpro/jwpl/api/Wikipedia.java‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,19 @@ public class Wikipedia
5353
private static final Logger logger = LoggerFactory
5454
.getLogger(MethodHandles.lookup().lookupClass());
5555

56-
// Note well: The whitespace at the beginning of this constant is here on purpose. Do NOT remove
57-
// it!
58-
static final String SQL_COLLATION = " COLLATE utf8mb4_bin"; /* " COLLATE utf8_bin"; */
56+
/**
57+
* The native query {@link #existsPage(String)} runs to find the entries of a title, taking the
58+
* title as parameter {@code pName}. It yields the names of the entries found.
59+
* <p>
60+
* Whether the comparison is case-sensitive depends on the collation of the column, and MySQL
61+
* and MariaDB default to case-insensitive ones. An explicit binary collation in the query,
62+
* though, keeps these databases from using the index on the column, which turns every lookup
63+
* into a full scan. So the query compares in the collation of the column, and the caller keeps
64+
* only the entries whose name equals the title exactly. These are few: the case variants of
65+
* one title at most.
66+
*/
67+
static final String PAGE_NAMES_BY_NAME_QUERY = "select p.name from PageMapLine as p "
68+
+ "where p.name = :pName";
5969

6070
/**
6171
* Upper bound for the number of page ids bound into a single {@code in (..)} clause of
@@ -745,23 +755,14 @@ public boolean existsPage(String title) {
745755

746756
String encodedTitle = t.getWikiStyleTitle();
747757

748-
final String query = "select p.id from PageMapLine as p where p.name = :pName"
749-
+ (dbConfig.supportsCollation() ? SQL_COLLATION : "");
750-
751-
return __inTransaction(session -> {
752-
// Eclipse somehow thinks that setParameter returns a MutationQuery instead of a
753-
// NativeQuery...
754-
// A name is not unique in PageMapLine: case variants of one title map to their own
755-
// entry, and a database whose charset cannot represent a title stores the substituted
756-
// characters, which lets unrelated titles collapse onto one name. Asking for a unique
757-
// result made this method throw NonUniqueResultException - unchecked, and out of a
758-
// method that promises a boolean instead of an exception. One entry is all it takes to
759-
// answer the question.
760-
var nativeQuery = session.createNativeQuery(query, Long.class)
761-
.setParameter("pName", encodedTitle, String.class)
762-
.setMaxResults(1);
763-
return nativeQuery.uniqueResult();
764-
}) != null;
758+
// A name is not unique in PageMapLine: case variants of one title map to their own
759+
// entry, and a database whose charset cannot represent a title stores the substituted
760+
// characters, which lets unrelated titles collapse onto one name. So all entries found
761+
// are fetched, and one of them matching the title exactly is all it takes.
762+
List<String> names = __inTransaction(
763+
session -> session.createNativeQuery(PAGE_NAMES_BY_NAME_QUERY, String.class)
764+
.setParameter("pName", encodedTitle, String.class).list());
765+
return names.contains(encodedTitle);
765766
}
766767

767768
/**

‎dkpro-jwpl-api/src/test/java/org/dkpro/jwpl/api/CategoryTest.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Collections;
2929
import java.util.HashSet;
3030
import java.util.List;
31+
import java.util.Locale;
3132
import java.util.Set;
3233
import java.util.UUID;
3334

@@ -342,11 +343,23 @@ public void testCreateCategoryByName()
342343
}
343344
}
344345

346+
/**
347+
* Category lookups are exact: a case variant of a name that has no entry of its own does not
348+
* denote a category, whatever the collation of the name column.
349+
*/
350+
@Test
351+
public void testCreateCategoryByNameCaseVariant()
352+
{
353+
assertThrows(WikiPageNotFoundException.class,
354+
() -> new Category(wiki, A_FAMOUS_CATEGORY.toLowerCase(Locale.ROOT)));
355+
assertThrows(WikiPageNotFoundException.class,
356+
() -> new Category(wiki, A_FAMOUS_CATEGORY.toUpperCase(Locale.ROOT)));
357+
}
358+
345359
@Test
346360
public void testCreateCategoryByNameIsCaseSensitive()
347361
{
348-
// the name lookup uses a binary collation where supported, so a variant that only
349-
// differs in case must not match
362+
// the name lookup is exact, so a variant that only differs in case must not match
350363
assertThrows(WikiPageNotFoundException.class, () -> new Category(wiki, "People of ukp"));
351364
}
352365

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Licensed to the Technische Universität Darmstadt under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The Technische Universität Darmstadt
6+
* licenses this file to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License.
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.dkpro.jwpl.api;
19+
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertTrue;
22+
23+
import java.sql.Connection;
24+
import java.sql.DriverManager;
25+
import java.sql.PreparedStatement;
26+
import java.sql.ResultSet;
27+
import java.sql.SQLException;
28+
29+
import org.dkpro.jwpl.api.testdb.JwplTestDatabase;
30+
import org.junit.jupiter.api.Test;
31+
import org.junit.jupiter.api.condition.EnabledIf;
32+
33+
/**
34+
* Verifies that the exact, case-sensitive name lookups of {@link Wikipedia#existsPage(String)} and
35+
* {@link Category#Category(Wikipedia, String)} can use the name indexes on MySQL and MariaDB. An
36+
* explicit collation in the comparison, e.g. {@code COLLATE utf8mb4_bin}, would force a full scan
37+
* instead.
38+
*/
39+
@EnabledIf("isMySqlFamily")
40+
public class NameLookupIndexTest
41+
extends BaseJWPLTest
42+
{
43+
44+
static boolean isMySqlFamily()
45+
{
46+
return JwplTestDatabase.selectEngine() != JwplTestDatabase.Engine.HSQLDB;
47+
}
48+
49+
@Test
50+
public void testExistsPageQueryUsesNameIndex() throws SQLException
51+
{
52+
assertIndexLookup(Wikipedia.PAGE_NAMES_BY_NAME_QUERY,
53+
"Exploring_the_Potential_of_Semantic_Relatedness_in_Information_Retrieval",
54+
"name_index");
55+
}
56+
57+
@Test
58+
public void testCategoryQueryUsesNameIndex() throws SQLException
59+
{
60+
assertIndexLookup(Category.CATEGORY_BY_NAME_QUERY, "People_of_UKP", "nameIndex");
61+
}
62+
63+
private static void assertIndexLookup(String query, String name, String index)
64+
throws SQLException
65+
{
66+
DatabaseConfiguration db = obtainDbConfiguration();
67+
String sql = "EXPLAIN " + query.replaceAll(":\\w+", "?");
68+
try (Connection c = DriverManager.getConnection(db.getJdbcURL(), db.getUser(),
69+
db.getPassword());
70+
PreparedStatement ps = c.prepareStatement(sql)) {
71+
long parameters = sql.chars().filter(ch -> ch == '?').count();
72+
for (int i = 1; i <= parameters; i++) {
73+
ps.setString(i, name);
74+
}
75+
try (ResultSet rs = ps.executeQuery()) {
76+
assertTrue(rs.next(), "EXPLAIN returned no plan for: " + sql);
77+
assertEquals("ref", rs.getString("type"), sql);
78+
assertEquals(index, rs.getString("key"), sql);
79+
}
80+
}
81+
}
82+
}

‎dkpro-jwpl-api/src/test/java/org/dkpro/jwpl/api/WikipediaTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.lang.invoke.MethodHandles;
2929
import java.util.Collections;
3030
import java.util.List;
31+
import java.util.Locale;
3132
import java.util.Map;
3233
import java.util.Set;
3334
import java.util.UUID;
@@ -264,6 +265,17 @@ public void testExistsPageByTitleInvalid()
264265
assertFalse(wiki.existsPage(" (X)"));
265266
}
266267

268+
/**
269+
* Title lookups are exact: a case variant of a title that has no entry of its own does not
270+
* exist, whatever the collation of the name column.
271+
*/
272+
@Test
273+
public void testExistsPageByTitleCaseVariant()
274+
{
275+
assertFalse(wiki.existsPage(A_FAMOUS_PAGE.replace("Potential", "potential")));
276+
assertFalse(wiki.existsPage(A_FAMOUS_PAGE.toUpperCase(Locale.ROOT)));
277+
}
278+
267279
@Test
268280
public void testExistsPageByTitleNullOrEmpty()
269281
{

0 commit comments

Comments
 (0)