[main] Issue #2301 - Sorting not honored with JPA and Oracle

foundation/org.eclipse.persistence.core.test.framework/src/main/java/org/eclipse/persistence/testing/framework/TogglingFastTableCreator.java

@@ -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
@@ -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.
@@ -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);
     }
 
     /**

foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/platform/database/OraclePlatform.java

@@ -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);

jpa/eclipselink.jpa.testapps/jpa.test.advanced/src/main/java/org/eclipse/persistence/testing/models/jpa/advanced/AdvancedTableCreator.java

@@ -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;
@@ -130,6 +131,7 @@ public AdvancedTableCreator() {
         addTableDefinition(buildPlanarbeitsgangTable());
         addTableDefinition(buildPlanarbeitsgangHistTable());
         addTableDefinition(buildMaterialReignisTable());
+        addTableDefinition(buildEntityFloatTable());
     }
 
     public TableDefinition buildADDRESSTable() {
@@ -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();

jpa/eclipselink.jpa.testapps/jpa.test.advanced/src/main/java/org/eclipse/persistence/testing/models/jpa/advanced/entities/EntityFloat.java

@@ -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);
+    }
+
+}

jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/main/resources/META-INF/persistence.xml

@@ -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>
@@ -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>

jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/AdvancedQueryTest.java

@@ -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;
 
@@ -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;
     }
@@ -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() {
@@ -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());
+    }
+
 }

jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/EntityFloatPopulator.java

@@ -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();
+    }
+
+}