This is an automated email from the ASF dual-hosted git repository. solomax pushed a commit to branch OPENJPA-2940-mysql9 in repository https://gitbox.apache.org/repos/asf/openjpa.git
commit 13cd75c983d1aec9ddd0c38054d1ee91ebc5cbf7 Author: Maxim Solodovnik <[email protected]> AuthorDate: Sun Jun 7 09:06:53 2026 +0700 [OPENJPA-2940] Reserved words are better handled --- .../apache/openjpa/jdbc/sql/MySQLDictionary.java | 16 +++++++-- .../apache/openjpa/jdbc/meta/TestSchemaTool.java | 28 +++++++++------ .../openjpa/lib/identifier/IdentifierRule.java | 8 +++-- .../criteria/AbstractCriteriaTestCase.java | 40 ++++++++++++++-------- .../persistence/jdbc/TestFKColumnNames.java | 21 +++++++++--- .../persistence/test/SQLListenerTestCase.java | 11 +++++- 6 files changed, 88 insertions(+), 36 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MySQLDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MySQLDictionary.java index 8d8676f1e..8a2398774 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MySQLDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MySQLDictionary.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Set; import org.apache.openjpa.jdbc.identifier.DBIdentifier; +import org.apache.openjpa.jdbc.identifier.Normalizer; import org.apache.openjpa.jdbc.identifier.DBIdentifier.DBIdentifierType; import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration; import org.apache.openjpa.jdbc.kernel.JDBCStore; @@ -41,6 +42,7 @@ import org.apache.openjpa.jdbc.schema.ForeignKey; import org.apache.openjpa.jdbc.schema.Index; import org.apache.openjpa.jdbc.schema.PrimaryKey; import org.apache.openjpa.jdbc.schema.Table; +import org.apache.openjpa.lib.identifier.IdentifierRule; import org.apache.openjpa.lib.util.StringUtil; import org.apache.openjpa.util.ExceptionInfo; import org.apache.openjpa.util.StoreException; @@ -129,7 +131,7 @@ public class MySQLDictionary "AUTO_INCREMENT", "BINARY", "BLOB", "CHANGE", "ENUM", "INFILE", "INT1", "INT2", "INT4", "FLOAT1", "FLOAT2", "FLOAT4", "LOAD", "MEDIUMINT", "OUTFILE", "REPLACE", "STARTING", "TEXT", "UNSIGNED", - "ZEROFILL", "INDEX", + "ZEROFILL", "INDEX", "LIBRARY" })); // reservedWordSet subset that CANNOT be used as valid column names @@ -271,6 +273,14 @@ public class MySQLDictionary return new int[]{maj, min}; } + @Override + protected void configureNamingRules() { + super.configureNamingRules(); + IdentifierRule rule = Normalizer.getNamingConfiguration().getDefaultIdentifierRule(); + rule.setDelimitReservedWords(true); + rule.setReservedWords(reservedWordSet); + } + @Override public String[] getCreateTableSQL(Table table) { String[] sql = super.getCreateTableSQL(table); @@ -477,8 +487,8 @@ public class MySQLDictionary if (state == ExceptionInfo.GENERAL && ex.getErrorCode() == 0 && ex.getSQLState() == null) { // look at the nested MySQL exception for more details SQLException sqle = ex.getNextException(); - if (sqle != null - && (sqle.toString().startsWith("com.mysql.jdbc.exceptions.MySQLTimeoutException") || + if (sqle != null + && (sqle.toString().startsWith("com.mysql.jdbc.exceptions.MySQLTimeoutException") || sqle.toString().startsWith("com.mysql.cj.jdbc.exceptions.MySQLTimeoutException"))) { if (conf != null && conf.getLockTimeout() != -1) { state = StoreException.LOCK; diff --git a/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestSchemaTool.java b/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestSchemaTool.java index eaf4cd913..bb2f4aa7e 100644 --- a/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestSchemaTool.java +++ b/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestSchemaTool.java @@ -19,10 +19,12 @@ package org.apache.openjpa.jdbc.meta; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; import org.apache.openjpa.jdbc.conf.JDBCConfiguration; import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl; @@ -39,20 +41,20 @@ public class TestSchemaTool { @Parameters(name = "{index}: {0} -> {1}") public static Collection<Object[]> data() { return Arrays.asList(new Object[][] { - {"", 0}, - {" ", 0}, - {"org/apache/openjpa/jdbc/meta/testScript1", 0}, - {"org/apache/openjpa/jdbc/meta/testScript2", 1}, - {"org/apache/openjpa/jdbc/meta/testScript3", 1}, - {"org/apache/openjpa/jdbc/meta/testScript4", 0}, - {"org/apache/openjpa/jdbc/meta/testScriptMulti1", 1}, - {"org/apache/openjpa/jdbc/meta/testScriptMulti2", 0}, + {"", List.of()}, + {" ", List.of()}, + {"org/apache/openjpa/jdbc/meta/testScript1", List.of()}, + {"org/apache/openjpa/jdbc/meta/testScript2", List.of("SELECT * FROM Customers WHERE Country = 'Germany'")}, + {"org/apache/openjpa/jdbc/meta/testScript3", List.of("SELECT * FROM Customers")}, + {"org/apache/openjpa/jdbc/meta/testScript4", List.of()}, + {"org/apache/openjpa/jdbc/meta/testScriptMulti1", List.of("SELECT * FROM MyTable")}, + {"org/apache/openjpa/jdbc/meta/testScriptMulti2", List.of()}, }); } @Parameter(0) public String sqlScript; @Parameter(1) - public Integer resultingLines; + public List<String> expected; @Test public void testExecuteScript() throws Exception { @@ -75,6 +77,12 @@ public class TestSchemaTool { }; tool.setScriptToExecute(sqlScript); tool.run(); - assertEquals(resultingLines.intValue(), sqlToRun.size()); + if (expected.size() != sqlToRun.size()) { + fail("Expected list wasn't found: \r\n expected" + expected + + "\r\n actual: \r\n" + sqlToRun); + } + for (int i = 0; i < expected.size(); ++i) { + assertEquals(expected.get(i), sqlToRun.get(i)); + } } } diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/identifier/IdentifierRule.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/identifier/IdentifierRule.java index a9c90d250..a1dde236b 100644 --- a/openjpa-lib/src/main/java/org/apache/openjpa/lib/identifier/IdentifierRule.java +++ b/openjpa-lib/src/main/java/org/apache/openjpa/lib/identifier/IdentifierRule.java @@ -19,7 +19,9 @@ package org.apache.openjpa.lib.identifier; import java.util.HashSet; +import java.util.Locale; import java.util.Set; +import java.util.stream.Collectors; import org.apache.openjpa.lib.util.StringUtil; @@ -123,7 +125,9 @@ public class IdentifierRule { } public void setReservedWords(Set<String> reservedWords) { - _reservedWords = reservedWords; + _reservedWords = reservedWords.stream() + .map(w -> w.toUpperCase(Locale.ROOT)) + .collect(Collectors.toSet()); } public Set<String> getReservedWords() { @@ -202,6 +206,6 @@ public class IdentifierRule { } public boolean isReservedWord(String identifier) { - return _reservedWords.contains(identifier); + return _reservedWords.contains(identifier.toUpperCase(Locale.ROOT)); } } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/AbstractCriteriaTestCase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/AbstractCriteriaTestCase.java index 8db426de4..85b35dd7c 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/AbstractCriteriaTestCase.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/AbstractCriteriaTestCase.java @@ -53,7 +53,7 @@ import junit.framework.TestCase; * @version $Rev$ $Date$ */ public abstract class AbstractCriteriaTestCase extends TestCase { - + private static final Logger logger = Logger.getLogger(AbstractCriteriaTestCase.class.getCanonicalName()); protected abstract SQLAuditor getAuditor(); @@ -156,6 +156,13 @@ public abstract class AbstractCriteriaTestCase extends TestCase { executeAndCompareSQL(jpql, cQ, jQ, expectedSQL); } + protected boolean same(String expected, String sql) { + return sql.equalsIgnoreCase(expected) || + sql.replace(dict.getLeadingDelimiter(), "") + .replace(dict.getTrailingDelimiter(), "") + .equalsIgnoreCase(expected); + } + /** * Execute the two given queries. The first query originated from a JPQL string must be well-formed. The second * query originated from a Criteria is being tested. @@ -194,19 +201,20 @@ public abstract class AbstractCriteriaTestCase extends TestCase { return; for (int i = 0; i < jSQL.size(); i++) { - if (!jSQL.get(i).equalsIgnoreCase(cSQL.get(i))) { + boolean eq = same(cSQL.get(i), jSQL.get(i)); + if (!eq) { printSQL("Target SQL for JPQL", jSQL); printSQL("Target SQL for CriteriaQuery", cSQL); - assertTrue(i + "-th SQL for JPQL and CriteriaQuery for " + jpql + " is different\r\n" + - "JPQL = [" + jSQL.get(i) + "]\r\n" + - "CSQL = [" + cSQL.get(i) + "]\r\n", - jSQL.get(i).equalsIgnoreCase(cSQL.get(i))); } + assertTrue(i + "-th SQL for JPQL and CriteriaQuery for " + jpql + " is different\r\n" + + "JPQL = [" + jSQL.get(i) + "]\r\n" + + "CSQL = [" + cSQL.get(i) + "]\r\n", + eq); } if (expectedSQL != null) { assertTrue("SQL for JPQL " + jpql + " is different than expecetd " + expectedSQL, - jSQL.get(0).equalsIgnoreCase(expectedSQL)); + same(expectedSQL, jSQL.get(0))); } } @@ -227,12 +235,12 @@ public abstract class AbstractCriteriaTestCase extends TestCase { return; for (int i = 0; i < jSQL.size(); i++) { - if (!jSQL.get(i).equalsIgnoreCase(expectedSQL)) { + boolean eq = same(expectedSQL, jSQL.get(i)); + if (!eq) { printSQL("SQL for JPQL", jSQL.get(i)); printSQL("Expected SQL", expectedSQL); - assertTrue(i + "-th SQL for JPQL: " + jSQL.get(i) + " are different than Expected SQL " + expectedSQL, - expectedSQL.equalsIgnoreCase(jSQL.get(i))); } + assertTrue(i + "-th SQL for JPQL: " + jSQL.get(i) + " are different than Expected SQL " + expectedSQL, eq); } } @@ -246,18 +254,20 @@ public abstract class AbstractCriteriaTestCase extends TestCase { fail(w.toString()); } - if (!(dict instanceof DerbyDictionary || dict instanceof MySQLDictionary || dict instanceof MariaDBDictionary)) + if (!(dict instanceof DerbyDictionary || dict instanceof MySQLDictionary || dict instanceof MariaDBDictionary)) { return; + } String jSql = jSQL.get(0).trim(); - if (jSql.indexOf("optimize for 1 row") != -1) + if (jSql.indexOf("optimize for 1 row") != -1) { jSql = jSql.substring(0, jSql.indexOf("optimize for 1 row")).trim(); + } - if (!jSql.equalsIgnoreCase(expectedSQL)) { + boolean eq = same(expectedSQL, jSql); + if (!eq) { printSQL("SQL for JPQL", jSql); - assertTrue("SQL for JPQL " + jSql + " is different than expecetd " + expectedSQL, - expectedSQL.equalsIgnoreCase(jSql)); } + assertTrue("SQL for JPQL " + jSql + " is different than expecetd " + expectedSQL, eq); } void executeExpectFail(CriteriaQuery<?> c, String jpql) { diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestFKColumnNames.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestFKColumnNames.java index 2272e9dcd..492ffee3a 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestFKColumnNames.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestFKColumnNames.java @@ -20,7 +20,12 @@ package org.apache.openjpa.persistence.jdbc; import jakarta.persistence.Persistence; +import java.util.Locale; + +import org.apache.openjpa.jdbc.identifier.Normalizer; import org.apache.openjpa.jdbc.meta.MappingRepository; +import org.apache.openjpa.jdbc.schema.Column; +import org.apache.openjpa.lib.identifier.IdentifierConfiguration; import org.apache.openjpa.persistence.OpenJPAEntityManagerFactorySPI; import org.apache.openjpa.persistence.test.AbstractPersistenceTestCase; @@ -28,7 +33,13 @@ import org.apache.openjpa.persistence.test.AbstractPersistenceTestCase; * Testcase that verifies the names for Foreign Key columns is as expected. */ public class TestFKColumnNames extends AbstractPersistenceTestCase { - + private String getName(Column col) { + IdentifierConfiguration cfg = Normalizer.getNamingConfiguration(); + return col.getIdentifier().getName() + .replace(cfg.getLeadingDelimiter(), "") + .replace(cfg.getTrailingDelimiter(), "") + .toUpperCase(Locale.ROOT); + } /** * <P> @@ -48,11 +59,11 @@ public class TestFKColumnNames extends AbstractPersistenceTestCase { (MappingRepository) emf.getConfiguration() .getMetaDataRepositoryInstance(); - assertEquals("SELECT_ID", repos.getMapping(FKColumnNamesInner1Entity.class, null, true) - .getFieldMapping("select").getColumns()[0].getName()); + assertEquals("SELECT_ID", getName(repos.getMapping(FKColumnNamesInner1Entity.class, null, true) + .getFieldMapping("select").getColumns()[0])); - assertEquals("FROM_ID", repos.getMapping(FKColumnNamesInner2Entity.class, null, true) - .getFieldMapping("from").getColumns()[0].getName()); + assertEquals("FROM_ID", getName(repos.getMapping(FKColumnNamesInner2Entity.class, null, true) + .getFieldMapping("from").getColumns()[0])); closeEMF(emf); } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java index 778d50daf..4186ade2c 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SQLListenerTestCase.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.apache.openjpa.jdbc.sql.DBDictionary; import org.apache.openjpa.lib.jdbc.JDBCListener; /** @@ -57,9 +58,17 @@ public abstract class SQLListenerTestCase * @param sqlExp the SQL expression. E.g., "SELECT FOO .*" */ public void assertSQL(String sqlExp) { + DBDictionary dict = getDBDictionary(); for (String statement : sql) { - if (statement.matches(sqlExp)) + if (statement.matches(sqlExp)) { + return; + } + String noDelims = statement + .replace(dict.getLeadingDelimiter(), "") + .replace(dict.getTrailingDelimiter(), ""); + if (noDelims.matches(sqlExp)) { return; + } } fail("Expected regular expression\r\n <" + sqlExp
