HHH-18033 Fix LimitHandler detect wrong statement end if sql contains quoted semicolon

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/IngresLimitHandler.java

@@ -66,7 +66,7 @@ public class IngresLimitHandler extends OffsetFetchLimitHandler {
 	};
 
 	private static final Pattern WITH_OPTION_PATTERN =
-			Pattern.compile("\\s+with\\s+(" + String.join("|", WITH_OPTIONS) + ")\\b|\\s*(;|$)");
+			Pattern.compile("\\s+with\\s+(" + String.join("|", WITH_OPTIONS) + ")\\b|\\s*;?\\s*$");
 
 	/**
 	 * The offset/fetch clauses must come before

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/RowsLimitHandler.java

@@ -47,7 +47,7 @@ public class RowsLimitHandler extends AbstractSimpleLimitHandler {
 	}
 
 	private static final Pattern FOR_UPDATE_PATTERN =
-			compile("\\s+for\\s+update\\b|\\s+with\\s+lock\\b|\\s*(;|$)", CASE_INSENSITIVE);
+			compile("\\s+for\\s+update\\b|\\s+with\\s+lock\\b|\\s*;?\\s*$", CASE_INSENSITIVE);
 
 	@Override
 	protected Pattern getForUpdatePattern() {

hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/IngresLimitHandlerTest.java

@@ -0,0 +1,22 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.community.dialect;
+
+import org.hibernate.community.dialect.pagination.IngresLimitHandler;
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+import org.hibernate.orm.test.dialect.AbstractLimitHandlerTest;
+
+/**
+ * @author Yanming Zhou
+ */
+public class IngresLimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return IngresLimitHandler.INSTANCE;
+	}
+}

hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/RowsLimitHandlerTest.java

@@ -0,0 +1,27 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.community.dialect;
+
+import org.hibernate.community.dialect.pagination.RowsLimitHandler;
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+import org.hibernate.orm.test.dialect.AbstractLimitHandlerTest;
+
+/**
+ * @author Yanming Zhou
+ */
+public class RowsLimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return RowsLimitHandler.INSTANCE;
+	}
+
+	@Override
+	protected String getLimitClause() {
+		return " rows ?";
+	}
+}

hibernate-core/src/main/java/org/hibernate/dialect/pagination/AbstractLimitHandler.java

@@ -33,10 +33,10 @@ public abstract class AbstractLimitHandler implements LimitHandler {
 			compile( "^\\s*select(\\s+(distinct|all))?\\b", CASE_INSENSITIVE );
 
 	private static final Pattern END_PATTERN =
-			compile("\\s*(;|$)", CASE_INSENSITIVE);
+			compile("\\s*;?\\s*$", CASE_INSENSITIVE);
 
 	private static final Pattern FOR_UPDATE_PATTERN =
-			compile("\\s+for\\s+update\\b|\\s*(;|$)", CASE_INSENSITIVE);
+			compile("\\s+for\\s+update\\b|\\s*;?\\s*$", CASE_INSENSITIVE);
 
 
 	@Override

hibernate-core/src/main/java/org/hibernate/dialect/pagination/DerbyLimitHandler.java

@@ -29,7 +29,7 @@ public class DerbyLimitHandler extends OffsetFetchLimitHandler {
 	}
 
 	private static final Pattern FOR_UPDATE_WITH_LOCK_PATTERN =
-			Pattern.compile("\\s+for\\s+(update|read|fetch)\\b|\\s+with\\s+(rr|rs|cs|ur)\\b|\\s*(;|$)");
+			Pattern.compile("\\s+for\\s+(update|read|fetch)\\b|\\s+with\\s+(rr|rs|cs|ur)\\b|\\s*;?\\s*$");
 
 	/**
 	 * The offset/fetch clauses must come before the

hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitLimitHandler.java

@@ -34,7 +34,7 @@ public class LimitLimitHandler extends AbstractSimpleLimitHandler {
 	}
 
 	private static final Pattern FOR_UPDATE_PATTERN =
-			compile("\\s+for\\s+update\\b|\\s+lock\\s+in\\s+shared\\s+mode\\b|\\s*(;|$)", CASE_INSENSITIVE);
+			compile("\\s+for\\s+update\\b|\\s+lock\\s+in\\s+shared\\s+mode\\b|\\s*;?\\s*$", CASE_INSENSITIVE);
 
 	@Override
 	protected Pattern getForUpdatePattern() {

hibernate-core/src/main/java/org/hibernate/dialect/pagination/Oracle12LimitHandler.java

@@ -99,7 +99,7 @@ public class Oracle12LimitHandler extends AbstractLimitHandler {
 			offsetFetchString = " fetch first ? rows only";
 		}
 
-		return sql + offsetFetchString;
+		return insertAtEnd(offsetFetchString, sql);
 	}
 
 	/**

hibernate-core/src/test/java/org/hibernate/orm/test/dialect/AbstractLimitHandlerTest.java

@@ -0,0 +1,73 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.dialect;
+
+import org.hibernate.dialect.pagination.LimitHandler;
+import org.hibernate.dialect.pagination.OffsetFetchLimitHandler;
+import org.hibernate.query.spi.Limit;
+import org.hibernate.query.spi.QueryOptions;
+import org.junit.jupiter.api.Test;
+
+import static org.hibernate.dialect.pagination.AbstractLimitHandler.hasFirstRow;
+import static org.hibernate.dialect.pagination.AbstractLimitHandler.hasMaxRows;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * @author Yanming Zhou
+ */
+public abstract class AbstractLimitHandlerTest {
+
+	@Test
+	public void testSqlWithSemicolonInsideQuotedString() {
+		String sql = "select * from Person p where p.name like ';'";
+		String expected = "select * from Person p where p.name like ';'" + getLimitClause();
+		assertGenerateExpectedSql(expected, sql);
+
+		sql = "select * from Person p where p.name like ';' ";
+		expected = "select * from Person p where p.name like ';'" + getLimitClause() + " ";
+		assertGenerateExpectedSql(expected, sql);
+	}
+
+	@Test
+	public void testSqlWithSemicolonInsideQuotedStringAndEndsWithSemicolon() {
+		String sql = "select * from Person p where p.name like ';';";
+		String expected = "select * from Person p where p.name like ';'" + getLimitClause() + ";";
+		assertGenerateExpectedSql(expected, sql);
+
+		sql = "select * from Person p where p.name like ';' ; ";
+		expected = "select * from Person p where p.name like ';'" + getLimitClause() + " ; ";
+		assertGenerateExpectedSql(expected, sql);
+	}
+
+	protected void assertGenerateExpectedSql(String expected, String sql) {
+		assertEquals(expected, getLimitHandler().processSql(sql, getLimit(), QueryOptions.NONE));
+	}
+
+	protected abstract LimitHandler getLimitHandler();
+
+	protected Limit getLimit() {
+		return new Limit(0, 10);
+	}
+
+	protected String getLimitClause() {
+		LimitHandler handler = getLimitHandler();
+		if (handler instanceof OffsetFetchLimitHandler) {
+			OffsetFetchLimitHandler oflh = (OffsetFetchLimitHandler) handler;
+			Limit limit = getLimit();
+			if (hasFirstRow(limit) && hasMaxRows(limit)) {
+				return " offset " + (oflh.supportsVariableLimit() ? "?" : String.valueOf(limit.getFirstRow()))
+						+ " rows fetch next " + (oflh.supportsVariableLimit() ? "?" : String.valueOf(limit.getMaxRows())) + " rows only";
+			}
+			else if (hasFirstRow(limit)) {
+				return " offset " + (oflh.supportsVariableLimit() ? "?" : String.valueOf(limit.getFirstRow())) + " rows";
+			} else {
+				return " fetch first " + (oflh.supportsVariableLimit() ? "?" : String.valueOf(limit.getMaxRows())) + " rows only";
+			}
+		}
+		return " limit ?";
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DB2LimitHandlerTest.java

@@ -0,0 +1,21 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.dialect;
+
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+import org.hibernate.dialect.pagination.DB2LimitHandler;
+
+/**
+ * @author Yanming Zhou
+ */
+public class DB2LimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return DB2LimitHandler.INSTANCE;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DerbyLimitHandlerTest.java

@@ -0,0 +1,21 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.dialect;
+
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+import org.hibernate.dialect.pagination.DerbyLimitHandler;
+
+/**
+ * @author Yanming Zhou
+ */
+public class DerbyLimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return DerbyLimitHandler.INSTANCE;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/dialect/LimitLimitHandlerTest.java

@@ -0,0 +1,21 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.dialect;
+
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+import org.hibernate.dialect.pagination.LimitLimitHandler;
+
+/**
+ * @author Yanming Zhou
+ */
+public class LimitLimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return LimitLimitHandler.INSTANCE;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/dialect/OffsetFetchLimitHandlerTest.java

@@ -0,0 +1,21 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.dialect;
+
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+import org.hibernate.dialect.pagination.OffsetFetchLimitHandler;
+
+/**
+ * @author Yanming Zhou
+ */
+public class OffsetFetchLimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return OffsetFetchLimitHandler.INSTANCE;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/dialect/Oracle12LimitHandlerTest.java

@@ -6,27 +6,45 @@
  */
 package org.hibernate.orm.test.dialect;
 
+import org.hibernate.dialect.pagination.AbstractLimitHandler;
+
 import org.hibernate.dialect.pagination.Oracle12LimitHandler;
 import org.hibernate.query.spi.Limit;
-import org.hibernate.query.spi.QueryOptions;
-
 import org.hibernate.testing.TestForIssue;
 
 import org.junit.jupiter.api.Test;
 
-import static org.junit.Assert.assertEquals;
+import static org.hibernate.dialect.pagination.AbstractLimitHandler.hasFirstRow;
+import static org.hibernate.dialect.pagination.AbstractLimitHandler.hasMaxRows;
 
 @TestForIssue( jiraKey = "HHH-14649")
-public class Oracle12LimitHandlerTest {
+public class Oracle12LimitHandlerTest extends AbstractLimitHandlerTest {
+
+	@Override
+	protected AbstractLimitHandler getLimitHandler() {
+		return Oracle12LimitHandler.INSTANCE;
+	}
+
+	@Override
+	protected String getLimitClause() {
+		Limit limit = getLimit();
+		if ( hasFirstRow(limit) && hasMaxRows(limit) ) {
+			return " offset ? rows fetch next ? rows only";
+		}
+		else if ( hasFirstRow(limit) ) {
+			return " offset ? rows";
+		}
+		else {
+			return " fetch first ? rows only";
+		}
+	}
 
 	@Test
 	public void testSqlWithSpace() {
 		final String sql = "select  p.name from Person p where p.id = 1 for update";
 		final String expected = "select * from (select  p.name from Person p where p.id = 1) where rownum<=? for update";
 
-		final String processedSql = Oracle12LimitHandler.INSTANCE.processSql( sql, new Limit( 0, 5 ), QueryOptions.NONE );
+		assertGenerateExpectedSql(expected, sql);
-
-		assertEquals( expected, processedSql );
 	}
 
 	@Test
@@ -34,9 +52,7 @@ public class Oracle12LimitHandlerTest {
 		final String sql = "select p.name from Person p where p.name =  ' this is a  string with spaces  ' for update";
 		final String expected = "select * from (select p.name from Person p where p.name =  ' this is a  string with spaces  ') where rownum<=? for update";
 
-		final String processedSql = Oracle12LimitHandler.INSTANCE.processSql( sql, new Limit( 0, 5 ), QueryOptions.NONE );
+		assertGenerateExpectedSql(expected, sql);
-
-		assertEquals( expected, processedSql );
 	}
 
 	@Test
@@ -44,9 +60,7 @@ public class Oracle12LimitHandlerTest {
 		final String sql = "select a.prop from A a where a.name =  'this is for update '";
 		final String expected = "select a.prop from A a where a.name =  'this is for update ' fetch first ? rows only";
 
-		final String processedSql = Oracle12LimitHandler.INSTANCE.processSql( sql, new Limit( 0, 5 ), QueryOptions.NONE );
+		assertGenerateExpectedSql(expected, sql);
-
-		assertEquals( expected, processedSql );
 	}
 
 	@Test
@@ -54,9 +68,7 @@ public class Oracle12LimitHandlerTest {
 		final String sql = "select a.prop from A a where a.name =  'this is for update ' for update";
 		final String expected = "select * from (select a.prop from A a where a.name =  'this is for update ') where rownum<=? for update";
 
-		final String processedSql = Oracle12LimitHandler.INSTANCE.processSql( sql, new Limit( 0, 5 ), QueryOptions.NONE );
+		assertGenerateExpectedSql(expected, sql);
-
-		assertEquals( expected, processedSql );
 	}
 
 }

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/query/QueryTest.java

@@ -28,8 +28,11 @@ import org.hibernate.Hibernate;
 import org.hibernate.QueryException;
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.dialect.CockroachDialect;
+import org.hibernate.dialect.DerbyDialect;
+import org.hibernate.dialect.OracleDialect;
 import org.hibernate.dialect.PostgreSQLDialect;
 import org.hibernate.dialect.PostgresPlusDialect;
+import org.hibernate.dialect.SybaseDialect;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.orm.test.jpa.BaseEntityManagerFunctionalTestCase;
 import org.hibernate.orm.test.jpa.Distributor;
@@ -40,6 +43,7 @@ import org.hibernate.stat.Statistics;
 import org.hibernate.testing.SkipForDialect;
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.orm.junit.Jira;
+import org.hibernate.testing.orm.junit.JiraKey;
 import org.junit.Test;
 import junit.framework.Assert;
 
@@ -55,6 +59,7 @@ import static org.junit.Assert.fail;
  * @author Emmanuel Bernard
  * @author Steve Ebersole
  * @author Chris Cranford
+ * @author Yanming Zhou
  */
 public class QueryTest extends BaseEntityManagerFunctionalTestCase {
 	@Override
@@ -546,6 +551,74 @@ public class QueryTest extends BaseEntityManagerFunctionalTestCase {
 		}
 	}
 
+	@Test
+	@JiraKey("HHH-18033")
+	public void testQueryContainsQuotedSemicolonWithLimit() {
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+		try {
+			em.persist( new Item( "Mouse;", "Micro$oft mouse" ) );
+
+			Query q = em.createQuery( "from Item where name like '%;%'" ).setMaxResults(10);
+			assertEquals( 1, q.getResultList().size() );
+
+			q = em.createQuery( "from Item where name like '%;%' " ).setMaxResults(10);
+			assertEquals( 1, q.getResultList().size() );
+		}
+		finally {
+			if ( em.getTransaction() != null && em.getTransaction().isActive() ) {
+				em.getTransaction().rollback();
+			}
+			em.close();
+		}
+	}
+
+	@Test
+	@JiraKey("HHH-18033")
+	public void testNativeQueryContainsQuotedSemicolonWithLimit() {
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+		try {
+			em.persist( new Item( "Mouse;", "Micro$oft mouse" ) );
+
+			Query q = em.createNativeQuery( "select * from Item where name like '%;%'" ).setMaxResults(10);
+			assertEquals( 1, q.getResultList().size() );
+
+			q = em.createNativeQuery( "select * from Item where name like '%;%' " ).setMaxResults(10);
+			assertEquals( 1, q.getResultList().size() );
+		}
+		finally {
+			if ( em.getTransaction() != null && em.getTransaction().isActive() ) {
+				em.getTransaction().rollback();
+			}
+			em.close();
+		}
+	}
+
+	@Test
+	@SkipForDialect(value = OracleDialect.class, jiraKey = "HHH-18033", comment = "Doesn't support semicolon as ending of statement")
+	@SkipForDialect(value = SybaseDialect.class, jiraKey = "HHH-18033", comment = "Doesn't support semicolon as ending of statement")
+	@SkipForDialect(value = DerbyDialect.class, jiraKey = "HHH-18033", comment = "Doesn't support semicolon as ending of statement")
+	public void testNativeQueryContainsQuotedSemicolonAndEndsWithSemicolonWithLimit() {
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+		try {
+			em.persist( new Item( "Mouse;", "Micro$oft mouse" ) );
+
+			Query q = em.createNativeQuery( "select * from Item where name like '%;%';" ).setMaxResults(10);
+			assertEquals( 1, q.getResultList().size() );
+
+			q = em.createNativeQuery( "select * from Item where name like '%;%' ; " ).setMaxResults(10);
+			assertEquals( 1, q.getResultList().size() );
+		}
+		finally {
+			if ( em.getTransaction() != null && em.getTransaction().isActive() ) {
+				em.getTransaction().rollback();
+			}
+			em.close();
+		}
+	}
+
 	@Test
 	public void testAggregationReturnType() throws Exception {
 		EntityManager em = getOrCreateEntityManager();