Skip to content

Commit

Permalink
Cannot set FOR UPDATE in conjunction with ORDER BY and / or LIMIT (Or…
Browse files Browse the repository at this point in the history 
…acleDB & Derby) - bugfix + unit test (#1390)


Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
  • Loading branch information
@rfelcman
rfelcman committed Jan 21, 2022
1 parent f23a167 commit adf28f4
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 69 deletions.
129 changes: 81 additions & 48 deletions ...e.core/src/main/java/org/eclipse/persistence/internal/expressions/SQLSelectStatement.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2022 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
Expand Down Expand Up @@ -1725,66 +1725,99 @@ protected void normalizeOrderBy(Expression builder, List<Expression> allExpressi
public Vector<DatabaseField> printSQL(ExpressionSQLPrinter printer) {
try {
Vector<DatabaseField> selectFields = null;
printer.setRequiresDistinct(shouldDistinctBeUsed());
selectFields = printSQLSelect(printer);
printSQLWhereKeyWord(printer);
printSQLWhereClause(printer);
printSQLHierarchicalQueryClause(printer);
printSQLGroupByClause(printer);
printSQLHavingClause(printer);
printSQLOrderByClause(printer);
printSQLForUpdateClause(printer);
printSQLUnionClause(printer);
return selectFields;
} catch (IOException exception) {
throw ValidationException.fileError(exception);
}
}

if (hasUnionExpressions()) {
// Ensure union order using brackets.
int size = getUnionExpressions().size();
for (int index = 0; index < size; index++) {
printer.printString("(");
}
}
printer.printString("SELECT ");
public Vector<DatabaseField> printSQLSelect(ExpressionSQLPrinter printer) throws IOException {
Vector<DatabaseField> selectFields = null;
printer.setRequiresDistinct(shouldDistinctBeUsed());

if (getHintString() != null) {
printer.printString(getHintString());
printer.printString(" ");
if (hasUnionExpressions()) {
// Ensure union order using brackets.
int size = getUnionExpressions().size();
for (int index = 0; index < size; index++) {
printer.printString("(");
}
}
printer.printString("SELECT ");

if (shouldDistinctBeUsed()) {
printer.printString("DISTINCT ");
}
if (getHintString() != null) {
printer.printString(getHintString());
printer.printString(" ");
}

selectFields = writeFieldsIn(printer);
//fix bug:6070214: turn off unique field aliases after fields are written
setUseUniqueFieldAliases(false);
if (shouldDistinctBeUsed()) {
printer.printString("DISTINCT ");
}

appendFromClauseToWriter(printer);
selectFields = writeFieldsIn(printer);
//fix bug:6070214: turn off unique field aliases after fields are written
setUseUniqueFieldAliases(false);

if (!(getWhereClause() == null)) {
printer.printString(" WHERE ");
printer.printExpression(getWhereClause());
}
appendFromClauseToWriter(printer);
return selectFields;
}

if (hasHierarchicalQueryExpressions()) {
appendHierarchicalQueryClauseToWriter(printer);
}
public void printSQLWhereKeyWord(ExpressionSQLPrinter printer) throws IOException {
if (!(getWhereClause() == null)) {
printer.printString(" WHERE ");
}
}

if (hasGroupByExpressions()) {
appendGroupByClauseToWriter(printer);
}
if (hasHavingExpression()) {
//appendHavingClauseToWriter(printer);
printer.printString(" HAVING ");
printer.printExpression(getHavingExpression());
}
public void printSQLWhereClause(ExpressionSQLPrinter printer) throws IOException {
if (!(getWhereClause() == null)) {
printer.printExpression(getWhereClause());
}
}

if (hasOrderByExpressions()) {
appendOrderClauseToWriter(printer);
}
public void printSQLHierarchicalQueryClause(ExpressionSQLPrinter printer) throws IOException {
if (hasHierarchicalQueryExpressions()) {
appendHierarchicalQueryClauseToWriter(printer);
}
}

if(printer.getPlatform().shouldPrintLockingClauseAfterWhereClause() && printer.getPlatform().shouldPrintForUpdateClause()) {
// For pessimistic locking.
appendForUpdateClause(printer);
}
public void printSQLGroupByClause(ExpressionSQLPrinter printer) throws IOException {
if (hasGroupByExpressions()) {
appendGroupByClauseToWriter(printer);
}
}

if (hasUnionExpressions()) {
appendUnionClauseToWriter(printer);
}
public void printSQLHavingClause(ExpressionSQLPrinter printer) throws IOException {
if (hasHavingExpression()) {
//appendHavingClauseToWriter(printer);
printer.printString(" HAVING ");
printer.printExpression(getHavingExpression());
}
}

return selectFields;
} catch (IOException exception) {
throw ValidationException.fileError(exception);
public void printSQLOrderByClause(ExpressionSQLPrinter printer) throws IOException {
if (hasOrderByExpressions()) {
appendOrderClauseToWriter(printer);
}
}

public void printSQLForUpdateClause(ExpressionSQLPrinter printer) throws IOException {
if(printer.getPlatform().shouldPrintLockingClauseAfterWhereClause() && printer.getPlatform().shouldPrintForUpdateClause()) {
// For pessimistic locking.
appendForUpdateClause(printer);
}
}

public void printSQLUnionClause(ExpressionSQLPrinter printer) throws IOException {
if (hasUnionExpressions()) {
appendUnionClauseToWriter(printer);
}
}

Expand Down
36 changes: 30 additions & 6 deletions ...sistence.core/src/main/java/org/eclipse/persistence/platform/database/OraclePlatform.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2022 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2021 IBM Corporation. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand Down Expand Up @@ -952,13 +952,15 @@ public boolean useJDBCStoredProcedureSyntax() {
protected String MIN_ROW = ") WHERE rnum > ";
// Bug #453208
protected String LOCK_START_PREFIX = " AND (";
protected String LOCK_START_PREFIX_WHERE = " WHERE (";
protected String LOCK_START_SUFFIX = ") IN (";
protected String LOCK_END = ") FOR UPDATE";
protected String LOCK_END = " FOR UPDATE";
protected String SELECT_ID_PREFIX = "SELECT ";
protected String SELECT_ID_SUFFIX = " FROM (SELECT ";
protected String FROM_ID = ", ROWNUM rnum FROM (";
protected String END_FROM_ID = ") ";
protected String ORDER_BY_ID = " ORDER BY ";
protected String BRACKET_END = " ) ";

/**
* INTERNAL:
Expand Down Expand Up @@ -989,7 +991,7 @@ public void printSQLSelectStatement(DatabaseCall call, ExpressionSQLPrinter prin
// Workaround can exist for this specific case
Vector fields = new Vector();
statement.enableFieldAliasesCaching();
String queryString = printOmittingForUpdate(statement, printer, fields);
String queryString = printOmittingOrderByForUpdateUnion(statement, printer, fields);
duplicateCallParameters(call);
call.setFields(fields);

Expand Down Expand Up @@ -1024,6 +1026,13 @@ public void printSQLSelectStatement(DatabaseCall call, ExpressionSQLPrinter prin
printer.printParameter(DatabaseCall.MAXROW_FIELD);
printer.printString(MIN_ROW);
printer.printParameter(DatabaseCall.FIRSTRESULT_FIELD);
printer.printString(BRACKET_END);
try {
statement.printSQLOrderByClause(printer);
statement.printSQLUnionClause(printer);
} catch (IOException exception) {
throw ValidationException.fileError(exception);
}
printer.printString(LOCK_END);
} else {
throw new UnsupportedOperationException(ExceptionLocalization.buildMessage("ora_pessimistic_locking_with_rownum"));
Expand Down Expand Up @@ -1067,14 +1076,25 @@ private void duplicateCallParameters(DatabaseCall call) {
}

@SuppressWarnings("unchecked")
private String printOmittingForUpdate(SQLSelectStatement statement, ExpressionSQLPrinter printer, Vector fields) {
private String printOmittingOrderByForUpdateUnion(SQLSelectStatement statement, ExpressionSQLPrinter printer, Vector fields) {
boolean originalShouldPrintForUpdate = this.shouldPrintForUpdateClause;
Writer originalWriter = printer.getWriter();
Vector selectFields = null;

this.shouldPrintForUpdateClause = false;
printer.setWriter(new StringWriter());

fields.addAll(statement.printSQL(printer));
try {
selectFields = statement.printSQLSelect(printer);
statement.printSQLWhereKeyWord(printer);
statement.printSQLWhereClause(printer);
statement.printSQLHierarchicalQueryClause(printer);
statement.printSQLGroupByClause(printer);
statement.printSQLHavingClause(printer);
} catch (IOException exception) {
throw ValidationException.fileError(exception);
}
fields.addAll(selectFields);
String query = printer.getWriter().toString();

this.shouldPrintForUpdateClause = originalShouldPrintForUpdate;
Expand All @@ -1084,7 +1104,11 @@ private String printOmittingForUpdate(SQLSelectStatement statement, ExpressionSQ
}

private void printLockStartWithPrimaryKeyFields(SQLSelectStatement statement, ExpressionSQLPrinter printer) {
printer.printString(LOCK_START_PREFIX);
if (statement.getWhereClause() == null) {
printer.printString(LOCK_START_PREFIX_WHERE);
} else {
printer.printString(LOCK_START_PREFIX);
}

Iterator<DatabaseField> iterator = statement.getQuery().getDescriptor().getPrimaryKeyFields().iterator();
while (iterator.hasNext()) {
Expand Down
66 changes: 51 additions & 15 deletions ...a.test.jse/src/it/java/org/eclipse/persistence/jpa/test/criteria/TestCriteriaBuilder.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2022 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021 IBM Corporation. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand All @@ -21,6 +21,7 @@

import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.LockModeType;
import jakarta.persistence.Query;
import jakarta.persistence.TypedQuery;
import jakarta.persistence.criteria.CompoundSelection;
Expand All @@ -42,23 +43,21 @@
import org.eclipse.persistence.jpa.test.framework.Emf;
import org.eclipse.persistence.jpa.test.framework.EmfRunner;
import org.eclipse.persistence.platform.database.DatabasePlatform;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;

@RunWith(EmfRunner.class)
public class TestCriteriaBuilder {

@Emf(createTables = DDLGen.DROP_CREATE, classes = { L1.class, L2.class, CriteriaCar.class })
private EntityManagerFactory emf;

/**
* Merging ElementCollections on Oracle fails when EclipseLink generates
* a DELETE SQL statement with a WHERE clause containing a CLOB.
*
*/
@Test
public void testCriteriaCompoundSelectionModel() throws Exception {
@Before
public void setup() {
//Populate the database
EntityManager em = emf.createEntityManager();
try {
Expand All @@ -69,10 +68,10 @@ public void testCriteriaCompoundSelectionModel() throws Exception {
L1 l1 = new L1(1, "L1-1", l2);
L1 l1_2 = new L1(2, "L1-2", l2_2);

em.persist(l2);
em.persist(l2_2);
em.persist(l1);
em.persist(l1_2);
em.merge(l2);
em.merge(l2_2);
em.merge(l1);
em.merge(l1_2);

em.getTransaction().commit();
em.clear();
Expand All @@ -84,8 +83,16 @@ public void testCriteriaCompoundSelectionModel() throws Exception {
em.close();
}
}
}

em = emf.createEntityManager();
/**
* Merging ElementCollections on Oracle fails when EclipseLink generates
* a DELETE SQL statement with a WHERE clause containing a CLOB.
*
*/
@Test
public void testCriteriaCompoundSelectionModel() throws Exception {
EntityManager em = emf.createEntityManager();
try {
//Test CriteriaBuilder
final CriteriaBuilder builder = em.getCriteriaBuilder();
Expand All @@ -100,7 +107,7 @@ public void testCriteriaCompoundSelectionModel() throws Exception {
List<L1Model> l1List = q.getResultList();
if (l1List != null && !l1List.isEmpty()) {
for (L1Model l1m : l1List) {
Assert.assertNotNull(l1m.getL2());
assertNotNull(l1m.getL2());
}
}
} finally {
Expand Down Expand Up @@ -205,6 +212,35 @@ public void testCriteriaBuilder_ParameterInSelectClause() throws Exception {
}
}

@Test
public void testCriteriaBuilder_WhereOrderByResultLimitPesimisticWriteClause() throws Exception {
EntityManager em = emf.createEntityManager();
try {
em.getTransaction().begin();
//"SELECT OBJECT(l1) FROM L1 l1 WHERE l1.id > 0 ORDER BY l1.id FOR UPDATE"
//with row limit firstResult=1 maxResults=10
final CriteriaBuilder criteriaBuilder = em.getCriteriaBuilder();
final CriteriaQuery<L1> criteriaQuery = criteriaBuilder.createQuery(L1.class);
Root<L1> root = criteriaQuery.from(L1.class);
criteriaQuery.where(criteriaBuilder.greaterThan(root.get("id"), 0));
criteriaQuery.orderBy(criteriaBuilder.asc(root.get(L1_.id)));
final TypedQuery<L1> query = em.createQuery(criteriaQuery);
query.setFirstResult(1);
query.setMaxResults(10);
query.setLockMode(LockModeType.PESSIMISTIC_WRITE);
final List<L1> results = query.getResultList();
assertNotNull(results);
assertEquals(1, results.size());
} finally {
if (em.getTransaction().isActive()) {
em.getTransaction().rollback();
}
if(em.isOpen()) {
em.close();
}
}
}

private DatabasePlatform getPlatform(EntityManagerFactory emf) {
return ((EntityManagerFactoryImpl)emf).getServerSession().getPlatform();
}
Expand Down

0 comments on commit adf28f4

Please sign in to comment.