Skip to content

Commit

Permalink
Sorting not honored with JPA and Oracle (#2309)
Browse files Browse the repository at this point in the history
Fixes Issue #2301 - Sorting not honored with JPA and Oracle

Signed-off-by: Tomáš Kraus <[email protected]>
  • Loading branch information
Tomas-Kraus authored Dec 5, 2024
1 parent 28ffe73 commit cb30d81
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -241,22 +241,37 @@ protected static FieldDefinition createNumericFk(
* with given name and size and without any additional constraints.
* @param name Column name.
* @param size Column numeric type size.
* @param subSize Column numeric type sub size.
* @param allowNull Allow {@code null} values for column.
* @return Initialized {@link FieldDefinition} instance.
*/
protected static FieldDefinition createNumericColumn(
final String name, final int size, final boolean allowNull) {
final String name, final int size, final int subSize, final boolean allowNull) {
final FieldDefinition field = new FieldDefinition();
field.setName(name);
field.setTypeName("NUMERIC");
field.setSize(size);
field.setSubSize(subSize);
field.setShouldAllowNull(allowNull);
field.setIsPrimaryKey(false);
field.setUnique(false);
field.setIsIdentity(false);
return field;
}

/**
* Helper method to create {@link FieldDefinition} instance for numeric column
* with given name and size and without any additional constraints.
* @param name Column name.
* @param size Column numeric type size.
* @param allowNull Allow {@code null} values for column.
* @return Initialized {@link FieldDefinition} instance.
*/
protected static FieldDefinition createNumericColumn(
final String name, final int size, final boolean allowNull) {
return createNumericColumn(name, size, 0, allowNull);
}

/**
* Helper method to create {@link FieldDefinition} instance for numeric column
* with given name, size of {@code 15}, with {@code null} value allowed and
Expand All @@ -269,6 +284,44 @@ protected static FieldDefinition createNumericColumn(
return createNumericColumn(name, 15, true);
}

/**
* Helper method to create {@link FieldDefinition} instance for numeric column
* with given name to store float type values.
* @param name Column name.
* @param allowNull Allow {@code null} values for column.
* @return Initialized {@link FieldDefinition} instance.
*/
protected static FieldDefinition createFloatColumn(
final String name, final boolean allowNull) {
final FieldDefinition field = new FieldDefinition();
field.setName(name);
field.setType(Float.class);
field.setShouldAllowNull(allowNull);
field.setIsPrimaryKey(false);
field.setUnique(false);
field.setIsIdentity(false);
return field;
}

/**
* Helper method to create {@link FieldDefinition} instance for numeric column
* with given name to store double type values.
* @param name Column name.
* @param allowNull Allow {@code null} values for column.
* @return Initialized {@link FieldDefinition} instance.
*/
protected static FieldDefinition createDoubleColumn(
final String name, final boolean allowNull) {
final FieldDefinition field = new FieldDefinition();
field.setName(name);
field.setType(Double.class);
field.setShouldAllowNull(allowNull);
field.setIsPrimaryKey(false);
field.setUnique(false);
field.setIsIdentity(false);
return field;
}

/**
* Helper method to create {@link FieldDefinition} instance for <code>DTYPE</code>
* column used for inheritance in model.
Expand Down Expand Up @@ -317,7 +370,7 @@ protected static FieldDefinition createStringColumn(
*/
protected static FieldDefinition createStringColumn(
final String name) {
return createStringColumn(name, 32, true);
return createStringColumn(name, 255, true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1119,8 +1119,16 @@ public void printSQLSelectStatement(DatabaseCall call, ExpressionSQLPrinter prin
printer.printString(primaryKeyFields);
printer.printString(FROM_ID);
printer.printString(queryString);
printer.printString(ORDER_BY_ID);
printer.printString(primaryKeyFields);
if (statement.hasOrderByExpressions()) {
try {
statement.printSQLOrderByClause(printer);
} catch (IOException exception) {
throw ValidationException.fileError(exception);
}
} else {
printer.printString(ORDER_BY_ID);
printer.printString(primaryKeyFields);
}
printer.printString(END_FROM_ID);
printer.printString(MAX_ROW);
printer.printParameter(DatabaseCall.MAXROW_FIELD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.eclipse.persistence.logging.SessionLogEntry;
import org.eclipse.persistence.sessions.DatabaseSession;
import org.eclipse.persistence.testing.framework.TogglingFastTableCreator;
import org.eclipse.persistence.testing.models.jpa.advanced.entities.EntityFloat;
import org.eclipse.persistence.tools.schemaframework.FieldDefinition;
import org.eclipse.persistence.tools.schemaframework.ForeignKeyConstraint;
import org.eclipse.persistence.tools.schemaframework.TableDefinition;
Expand Down Expand Up @@ -130,6 +131,7 @@ public AdvancedTableCreator() {
addTableDefinition(buildPlanarbeitsgangTable());
addTableDefinition(buildPlanarbeitsgangHistTable());
addTableDefinition(buildMaterialReignisTable());
addTableDefinition(buildEntityFloatTable());
}

public TableDefinition buildADDRESSTable() {
Expand Down Expand Up @@ -3649,7 +3651,19 @@ public TableDefinition buildMaterialReignisTable() {

return tabledefinition;
}


// Supported data types according to https://docs.oracle.com/cd/E19501-01/819-3659/gcmaz/
private static TableDefinition buildEntityFloatTable() {
TableDefinition table = new TableDefinition();
table.setName(EntityFloat.TABLE_NAME);
table.addField(createNumericPk("ID", 10));
table.addField(createFloatColumn("HEIGHT", false));
table.addField(createFloatColumn("LENGTH", false));
table.addField(createFloatColumn("WIDTH", false));
table.addField(createStringColumn("DESCRIPTION", 255,false));
return table;
}

@Override
public void replaceTables(DatabaseSession session) {
DatabasePlatform dbPlatform = session.getPlatform();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2024 IBM Corporation. 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
* http://www.eclipse.org/legal/epl-2.0,
* or the Eclipse Distribution License v. 1.0 which is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
*/
package org.eclipse.persistence.testing.models.jpa.advanced.entities;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Table;

// Based on reproduction scenario from issue #2301 (https://github.com/eclipse-ee4j/eclipselink/issues/2301)
@Entity
@Table(name = EntityFloat.TABLE_NAME)
public class EntityFloat {

public static final String TABLE_NAME = "ADV_ENTITY_FLOAT";

@Id
@Column(name = "ID")
private int id;

@Column(name = "HEIGHT")
private float height;

@Column(name = "LENGTH")
private float length;

@Column(name = "WIDTH")
private float width;

@Column(name = "DESCRIPTION")
private String description;

public EntityFloat() {
this(-1, 0f, 0f, 0f, null);
}

public EntityFloat(int id, float length, float width, float height, String description) {
this.id = id;
this.length = length;
this.width = width;
this.height = height;
this.description = description;
}

public int getId() {
return id;
}

public void setId(int id) {
this.id = id;
}

public float getHeight() {
return height;
}

public void setHeight(float height) {
this.height = height;
}

public float getLength() {
return length;
}

public void setLength(float length) {
this.length = length;
}

public float getWidth() {
return width;
}

public void setWidth(float width) {
this.width = width;
}

public String getDescription() {
return description;
}

public void setDescription(String description) {
this.description = description;
}

@Override
public String toString() {
return String.format(
"EntityFloat{ id=%d, length=%f, width=%f, height=%f, description=\"%s\"}",
id, length, width, height, description);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntyC</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntyD</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntyE</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntityFloat</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.order.select.where.Order</class>
<exclude-unlisted-classes>true</exclude-unlisted-classes>
<properties>
Expand Down Expand Up @@ -97,6 +98,7 @@
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntyC</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntyD</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntyE</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.entities.EntityFloat</class>
<class>org.eclipse.persistence.testing.models.jpa.advanced.order.select.where.Order</class>
<exclude-unlisted-classes>true</exclude-unlisted-classes>
<properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.eclipse.persistence.testing.models.jpa.advanced.Employee;
import org.eclipse.persistence.testing.models.jpa.advanced.Employee.Gender;
import org.eclipse.persistence.testing.models.jpa.advanced.EmployeePopulator;
import org.eclipse.persistence.testing.models.jpa.advanced.entities.EntityFloat;
import org.eclipse.persistence.testing.tests.jpa.jpql.JUnitDomainObjectComparer;
import org.junit.Assert;

Expand Down Expand Up @@ -162,6 +163,7 @@ public static Test suite() {
suite.addTest(new AdvancedQueryTest("testVersionChangeWithReadLock"));
suite.addTest(new AdvancedQueryTest("testVersionChangeWithWriteLock"));
suite.addTest(new AdvancedQueryTest("testNamedQueryAnnotationOverwritePersistenceXML"));
suite.addTest(new AdvancedQueryTest("testFloatSortWithPessimisticLock"));
suite.addTest(new AdvancedQueryTest("testTearDown"));
return suite;
}
Expand All @@ -184,6 +186,8 @@ public void testSetup() {
employeePopulator.buildExamples();
//Persist the examples in the database
employeePopulator.persistExample(session);
// EntityFloat instances to test issue #2301
EntityFloatPopulator.populate(session);
}

public void testTearDown() {
Expand Down Expand Up @@ -2707,4 +2711,27 @@ public void run() {
closeEntityManager(em);
}
}

// Based on reproduction scenario from issue #2301 (https://github.com/eclipse-ee4j/eclipselink/issues/2301)
public void testFloatSortWithPessimisticLock() {
EntityManager em = createEntityManager();
beginTransaction(em);
List<EntityFloat> entities;
try {
entities = em.createQuery("SELECT f FROM EntityFloat f WHERE (f.height < ?1) ORDER BY f.height DESC, f.length",
EntityFloat.class)
.setParameter(1, 8.0)
.setLockMode(LockModeType.PESSIMISTIC_WRITE) // Cause of issue
.setMaxResults(2)
.getResultList();
commitTransaction(em);
} catch (PersistenceException ex) {
rollbackTransaction(em);
throw ex;
}
assertEquals(2, entities.size());
assertEquals(70071, entities.get(0).getId());
assertEquals(70077, entities.get(1).getId());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2024 IBM Corporation. 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
* http://www.eclipse.org/legal/epl-2.0,
* or the Eclipse Distribution License v. 1.0 which is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
*/
package org.eclipse.persistence.testing.tests.jpa.jpql.advanced;

import java.util.Arrays;
import java.util.List;

import org.eclipse.persistence.sessions.Session;
import org.eclipse.persistence.sessions.UnitOfWork;
import org.eclipse.persistence.testing.models.jpa.advanced.entities.EntityFloat;

class EntityFloatPopulator {

private EntityFloatPopulator() {
throw new UnsupportedOperationException("No instances of EntityFloatPopulator are allowed");
}

static EntityFloat[] ENTITY_FLOAT = new EntityFloat[] {
// Tallest and smallest length
new EntityFloat(70071, 17.0f, 17.1f, 7.7f, "testOLGH28289#70071"),
// Tallest and largest length
new EntityFloat(70077, 77.0f, 17.7f, 7.7f, "testOLGH28289#70077"),
new EntityFloat(70007, 70.0f, 10.7f, 0.7f, "testOLGH28289#70007")
};

static void populate(Session session) {
List<Object> entities = Arrays.asList(ENTITY_FLOAT);
UnitOfWork unitOfWork = session.acquireUnitOfWork();
unitOfWork.removeAllReadOnlyClasses();
unitOfWork.registerAllObjects(entities);
unitOfWork.commit();
}

}

0 comments on commit cb30d81

Please sign in to comment.