Skip to content

Commit 13cd75c

Browse files
solomaxsolomax
committed
[OPENJPA-2940] Reserved words are better handled
1 parent 2d2e236 commit 13cd75c

6 files changed

Lines changed: 88 additions & 36 deletions

File tree

‎openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MySQLDictionary.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Set;
3232

3333
import org.apache.openjpa.jdbc.identifier.DBIdentifier;
34+
import org.apache.openjpa.jdbc.identifier.Normalizer;
3435
import org.apache.openjpa.jdbc.identifier.DBIdentifier.DBIdentifierType;
3536
import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
3637
import org.apache.openjpa.jdbc.kernel.JDBCStore;
@@ -41,6 +42,7 @@
4142
import org.apache.openjpa.jdbc.schema.Index;
4243
import org.apache.openjpa.jdbc.schema.PrimaryKey;
4344
import org.apache.openjpa.jdbc.schema.Table;
45+
import org.apache.openjpa.lib.identifier.IdentifierRule;
4446
import org.apache.openjpa.lib.util.StringUtil;
4547
import org.apache.openjpa.util.ExceptionInfo;
4648
import org.apache.openjpa.util.StoreException;
@@ -129,7 +131,7 @@ public MySQLDictionary() {
129131
"AUTO_INCREMENT", "BINARY", "BLOB", "CHANGE", "ENUM", "INFILE",
130132
"INT1", "INT2", "INT4", "FLOAT1", "FLOAT2", "FLOAT4", "LOAD",
131133
"MEDIUMINT", "OUTFILE", "REPLACE", "STARTING", "TEXT", "UNSIGNED",
132-
"ZEROFILL", "INDEX",
134+
"ZEROFILL", "INDEX", "LIBRARY"
133135
}));
134136

135137
// reservedWordSet subset that CANNOT be used as valid column names
@@ -271,6 +273,14 @@ private static int[] getMajorMinorVersions(String versionStr)
271273
return new int[]{maj, min};
272274
}
273275

276+
@Override
277+
protected void configureNamingRules() {
278+
super.configureNamingRules();
279+
IdentifierRule rule = Normalizer.getNamingConfiguration().getDefaultIdentifierRule();
280+
rule.setDelimitReservedWords(true);
281+
rule.setReservedWords(reservedWordSet);
282+
}
283+
274284
@Override
275285
public String[] getCreateTableSQL(Table table) {
276286
String[] sql = super.getCreateTableSQL(table);
@@ -477,8 +487,8 @@ protected int matchErrorState(Map<Integer,Set<String>> errorStates, SQLException
477487
if (state == ExceptionInfo.GENERAL && ex.getErrorCode() == 0 && ex.getSQLState() == null) {
478488
// look at the nested MySQL exception for more details
479489
SQLException sqle = ex.getNextException();
480-
if (sqle != null
481-
&& (sqle.toString().startsWith("com.mysql.jdbc.exceptions.MySQLTimeoutException") ||
490+
if (sqle != null
491+
&& (sqle.toString().startsWith("com.mysql.jdbc.exceptions.MySQLTimeoutException") ||
482492
sqle.toString().startsWith("com.mysql.cj.jdbc.exceptions.MySQLTimeoutException"))) {
483493
if (conf != null && conf.getLockTimeout() != -1) {
484494
state = StoreException.LOCK;

‎openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestSchemaTool.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
package org.apache.openjpa.jdbc.meta;
2020

2121
import static org.junit.Assert.assertEquals;
22+
import static org.junit.Assert.fail;
2223

2324
import java.util.ArrayList;
2425
import java.util.Arrays;
2526
import java.util.Collection;
27+
import java.util.List;
2628

2729
import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
2830
import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl;
@@ -39,20 +41,20 @@ public class TestSchemaTool {
3941
@Parameters(name = "{index}: {0} -> {1}")
4042
public static Collection<Object[]> data() {
4143
return Arrays.asList(new Object[][] {
42-
{"", 0},
43-
{" ", 0},
44-
{"org/apache/openjpa/jdbc/meta/testScript1", 0},
45-
{"org/apache/openjpa/jdbc/meta/testScript2", 1},
46-
{"org/apache/openjpa/jdbc/meta/testScript3", 1},
47-
{"org/apache/openjpa/jdbc/meta/testScript4", 0},
48-
{"org/apache/openjpa/jdbc/meta/testScriptMulti1", 1},
49-
{"org/apache/openjpa/jdbc/meta/testScriptMulti2", 0},
44+
{"", List.of()},
45+
{" ", List.of()},
46+
{"org/apache/openjpa/jdbc/meta/testScript1", List.of()},
47+
{"org/apache/openjpa/jdbc/meta/testScript2", List.of("SELECT * FROM Customers WHERE Country = 'Germany'")},
48+
{"org/apache/openjpa/jdbc/meta/testScript3", List.of("SELECT * FROM Customers")},
49+
{"org/apache/openjpa/jdbc/meta/testScript4", List.of()},
50+
{"org/apache/openjpa/jdbc/meta/testScriptMulti1", List.of("SELECT * FROM MyTable")},
51+
{"org/apache/openjpa/jdbc/meta/testScriptMulti2", List.of()},
5052
});
5153
}
5254
@Parameter(0)
5355
public String sqlScript;
5456
@Parameter(1)
55-
public Integer resultingLines;
57+
public List<String> expected;
5658

5759
@Test
5860
public void testExecuteScript() throws Exception {
@@ -75,6 +77,12 @@ protected boolean executeSQL(String[] sql) {
7577
};
7678
tool.setScriptToExecute(sqlScript);
7779
tool.run();
78-
assertEquals(resultingLines.intValue(), sqlToRun.size());
80+
if (expected.size() != sqlToRun.size()) {
81+
fail("Expected list wasn't found: \r\n expected" + expected
82+
+ "\r\n actual: \r\n" + sqlToRun);
83+
}
84+
for (int i = 0; i < expected.size(); ++i) {
85+
assertEquals(expected.get(i), sqlToRun.get(i));
86+
}
7987
}
8088
}

‎openjpa-lib/src/main/java/org/apache/openjpa/lib/identifier/IdentifierRule.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
package org.apache.openjpa.lib.identifier;
2020

2121
import java.util.HashSet;
22+
import java.util.Locale;
2223
import java.util.Set;
24+
import java.util.stream.Collectors;
2325

2426
import org.apache.openjpa.lib.util.StringUtil;
2527

@@ -123,7 +125,9 @@ public boolean isOnlyLettersDigitsUnderscores() {
123125
}
124126

125127
public void setReservedWords(Set<String> reservedWords) {
126-
_reservedWords = reservedWords;
128+
_reservedWords = reservedWords.stream()
129+
.map(w -> w.toUpperCase(Locale.ROOT))
130+
.collect(Collectors.toSet());
127131
}
128132

129133
public Set<String> getReservedWords() {
@@ -202,6 +206,6 @@ public boolean requiresDelimiters(String identifier) {
202206
}
203207

204208
public boolean isReservedWord(String identifier) {
205-
return _reservedWords.contains(identifier);
209+
return _reservedWords.contains(identifier.toUpperCase(Locale.ROOT));
206210
}
207211
}

‎openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/AbstractCriteriaTestCase.java‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
* @version $Rev$ $Date$
5454
*/
5555
public abstract class AbstractCriteriaTestCase extends TestCase {
56-
56+
5757
private static final Logger logger = Logger.getLogger(AbstractCriteriaTestCase.class.getCanonicalName());
5858

5959
protected abstract SQLAuditor getAuditor();
@@ -156,6 +156,13 @@ void assertEquivalence(QueryDecorator decorator, CriteriaQuery<?> c, String jpql
156156
executeAndCompareSQL(jpql, cQ, jQ, expectedSQL);
157157
}
158158

159+
protected boolean same(String expected, String sql) {
160+
return sql.equalsIgnoreCase(expected) ||
161+
sql.replace(dict.getLeadingDelimiter(), "")
162+
.replace(dict.getTrailingDelimiter(), "")
163+
.equalsIgnoreCase(expected);
164+
}
165+
159166
/**
160167
* Execute the two given queries. The first query originated from a JPQL string must be well-formed. The second
161168
* query originated from a Criteria is being tested.
@@ -194,19 +201,20 @@ void executeAndCompareSQL(String jpql, Query cQ, Query jQ, String expectedSQL) {
194201
return;
195202

196203
for (int i = 0; i < jSQL.size(); i++) {
197-
if (!jSQL.get(i).equalsIgnoreCase(cSQL.get(i))) {
204+
boolean eq = same(cSQL.get(i), jSQL.get(i));
205+
if (!eq) {
198206
printSQL("Target SQL for JPQL", jSQL);
199207
printSQL("Target SQL for CriteriaQuery", cSQL);
200-
assertTrue(i + "-th SQL for JPQL and CriteriaQuery for " + jpql + " is different\r\n" +
201-
"JPQL = [" + jSQL.get(i) + "]\r\n" +
202-
"CSQL = [" + cSQL.get(i) + "]\r\n",
203-
jSQL.get(i).equalsIgnoreCase(cSQL.get(i)));
204208
}
209+
assertTrue(i + "-th SQL for JPQL and CriteriaQuery for " + jpql + " is different\r\n" +
210+
"JPQL = [" + jSQL.get(i) + "]\r\n" +
211+
"CSQL = [" + cSQL.get(i) + "]\r\n",
212+
eq);
205213
}
206214

207215
if (expectedSQL != null) {
208216
assertTrue("SQL for JPQL " + jpql + " is different than expecetd " + expectedSQL,
209-
jSQL.get(0).equalsIgnoreCase(expectedSQL));
217+
same(expectedSQL, jSQL.get(0)));
210218

211219
}
212220
}
@@ -227,12 +235,12 @@ void executeAndCompareSQL(String jpql, String expectedSQL) {
227235
return;
228236

229237
for (int i = 0; i < jSQL.size(); i++) {
230-
if (!jSQL.get(i).equalsIgnoreCase(expectedSQL)) {
238+
boolean eq = same(expectedSQL, jSQL.get(i));
239+
if (!eq) {
231240
printSQL("SQL for JPQL", jSQL.get(i));
232241
printSQL("Expected SQL", expectedSQL);
233-
assertTrue(i + "-th SQL for JPQL: " + jSQL.get(i) + " are different than Expected SQL " + expectedSQL,
234-
expectedSQL.equalsIgnoreCase(jSQL.get(i)));
235242
}
243+
assertTrue(i + "-th SQL for JPQL: " + jSQL.get(i) + " are different than Expected SQL " + expectedSQL, eq);
236244
}
237245
}
238246

@@ -246,18 +254,20 @@ void executeAndCompareSQL(Query jQ, String expectedSQL) {
246254
fail(w.toString());
247255
}
248256

249-
if (!(dict instanceof DerbyDictionary || dict instanceof MySQLDictionary || dict instanceof MariaDBDictionary))
257+
if (!(dict instanceof DerbyDictionary || dict instanceof MySQLDictionary || dict instanceof MariaDBDictionary)) {
250258
return;
259+
}
251260

252261
String jSql = jSQL.get(0).trim();
253-
if (jSql.indexOf("optimize for 1 row") != -1)
262+
if (jSql.indexOf("optimize for 1 row") != -1) {
254263
jSql = jSql.substring(0, jSql.indexOf("optimize for 1 row")).trim();
264+
}
255265

256-
if (!jSql.equalsIgnoreCase(expectedSQL)) {
266+
boolean eq = same(expectedSQL, jSql);
267+
if (!eq) {
257268
printSQL("SQL for JPQL", jSql);
258-
assertTrue("SQL for JPQL " + jSql + " is different than expecetd " + expectedSQL,
259-
expectedSQL.equalsIgnoreCase(jSql));
260269
}
270+
assertTrue("SQL for JPQL " + jSql + " is different than expecetd " + expectedSQL, eq);
261271
}
262272

263273
void executeExpectFail(CriteriaQuery<?> c, String jpql) {

‎openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestFKColumnNames.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,26 @@
2020

2121
import jakarta.persistence.Persistence;
2222

23+
import java.util.Locale;
24+
25+
import org.apache.openjpa.jdbc.identifier.Normalizer;
2326
import org.apache.openjpa.jdbc.meta.MappingRepository;
27+
import org.apache.openjpa.jdbc.schema.Column;
28+
import org.apache.openjpa.lib.identifier.IdentifierConfiguration;
2429
import org.apache.openjpa.persistence.OpenJPAEntityManagerFactorySPI;
2530
import org.apache.openjpa.persistence.test.AbstractPersistenceTestCase;
2631

2732
/**
2833
* Testcase that verifies the names for Foreign Key columns is as expected.
2934
*/
3035
public class TestFKColumnNames extends AbstractPersistenceTestCase {
31-
36+
private String getName(Column col) {
37+
IdentifierConfiguration cfg = Normalizer.getNamingConfiguration();
38+
return col.getIdentifier().getName()
39+
.replace(cfg.getLeadingDelimiter(), "")
40+
.replace(cfg.getTrailingDelimiter(), "")
41+
.toUpperCase(Locale.ROOT);
42+
}
3243

3344
/**
3445
* <P>
@@ -48,11 +59,11 @@ public void testSQLKeywords() {
4859
(MappingRepository) emf.getConfiguration()
4960
.getMetaDataRepositoryInstance();
5061

51-
assertEquals("SELECT_ID", repos.getMapping(FKColumnNamesInner1Entity.class, null, true)
52-
.getFieldMapping("select").getColumns()[0].getName());
62+
assertEquals("SELECT_ID", getName(repos.getMapping(FKColumnNamesInner1Entity.class, null, true)
63+
.getFieldMapping("select").getColumns()[0]));
5364

54-
assertEquals("FROM_ID", repos.getMapping(FKColumnNamesInner2Entity.class, null, true)
55-
.getFieldMapping("from").getColumns()[0].getName());
65+
assertEquals("FROM_ID", getName(repos.getMapping(FKColumnNamesInner2Entity.class, null, true)
66+
.getFieldMapping("from").getColumns()[0]));
5667
closeEMF(emf);
5768
}
5869

‎openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Arrays;
2323
import java.util.List;
2424

25+
import org.apache.openjpa.jdbc.sql.DBDictionary;
2526
import org.apache.openjpa.lib.jdbc.JDBCListener;
2627

2728
/**
@@ -57,9 +58,17 @@ public void tearDown() throws Exception {
5758
* @param sqlExp the SQL expression. E.g., "SELECT FOO .*"
5859
*/
5960
public void assertSQL(String sqlExp) {
61+
DBDictionary dict = getDBDictionary();
6062
for (String statement : sql) {
61-
if (statement.matches(sqlExp))
63+
if (statement.matches(sqlExp)) {
64+
return;
65+
}
66+
String noDelims = statement
67+
.replace(dict.getLeadingDelimiter(), "")
68+
.replace(dict.getTrailingDelimiter(), "");
69+
if (noDelims.matches(sqlExp)) {
6270
return;
71+
}
6372
}
6473

6574
fail("Expected regular expression\r\n <" + sqlExp

0 commit comments

Comments
 (0)