Issue #2301 - Sorting not honored with JPA and Oracle (#2315)

Signed-off-by: Tomáš Kraus <tomas.kraus@oracle.com>

Author: Tomáš Kraus

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2024 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2010, 2015 IBM Corporation.
  * Copyright (c) 2010, 2015 Dies Koper (Fujitsu).
  *
@@ -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
@@ -271,6 +286,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.
@@ -319,7 +372,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/org/eclipse/persistence/platform/database/OraclePlatform.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2024 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 1998, 2023 IBM Corporation. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -1030,8 +1030,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.test/src/org/eclipse/persistence/testing/models/jpa/advanced/AdvancedTableCreator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2024 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 1998, 2019 IBM Corporation. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -124,6 +124,7 @@ public AdvancedTableCreator() {
         addTableDefinition(buildORD_ENTITYATable());
         addTableDefinition(buildORD_ENTITYZTable());
         addTableDefinition(buildMyTestEntityTable());
+        addTableDefinition(buildEntityFloatTable());
     }
 
     public TableDefinition buildADDRESSTable() {
@@ -3377,7 +3378,20 @@ public TableDefinition buildMyTestEntityTable() {
 
         return table;
     }
-    
+
+
+    // Supported data types according to https://docs.oracle.com/cd/E19501-01/819-3659/gcmaz/
+    private 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.test/src/org/eclipse/persistence/testing/models/jpa/advanced/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;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.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.test/src/org/eclipse/persistence/testing/models/jpa/advanced/EntityFloatPopulator.java

@@ -0,0 +1,43 @@
+/*
+ * 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;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.eclipse.persistence.sessions.Session;
+import org.eclipse.persistence.sessions.UnitOfWork;
+
+public 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")
+    };
+
+    public static void populate(Session session) {
+        List<Object> entities = Arrays.asList(ENTITY_FLOAT);
+        UnitOfWork unitOfWork = session.acquireUnitOfWork();
+        unitOfWork.removeAllReadOnlyClasses();
+        unitOfWork.registerAllObjects(entities);
+        unitOfWork.commit();
+    }
+
+}

jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/AdvancedQueryTestSuite.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2024 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2019 IBM Corporation. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -60,11 +60,13 @@
 import org.eclipse.persistence.testing.models.jpa.inheritance.Engineer;
 import org.eclipse.persistence.testing.models.jpa.inheritance.InheritancePopulator;
 import org.eclipse.persistence.testing.models.jpa.inheritance.InheritanceTableCreator;
+import org.eclipse.persistence.testing.models.jpa.advanced.Address;
 import org.eclipse.persistence.testing.models.jpa.advanced.Buyer;
 import org.eclipse.persistence.testing.models.jpa.advanced.Department;
 import org.eclipse.persistence.testing.models.jpa.advanced.Employee;
-import org.eclipse.persistence.testing.models.jpa.advanced.Address;
+import org.eclipse.persistence.testing.models.jpa.advanced.EntityFloat;
 import org.eclipse.persistence.testing.models.jpa.advanced.EmployeePopulator;
+import org.eclipse.persistence.testing.models.jpa.advanced.EntityFloatPopulator;
 import org.eclipse.persistence.testing.models.jpa.advanced.AdvancedTableCreator;
 import org.eclipse.persistence.testing.models.jpa.advanced.Employee.Gender;
 import org.eclipse.persistence.testing.models.jpa.inheritance.Person;
@@ -170,6 +172,7 @@ public static Test suite() {
             suite.addTest(new AdvancedQueryTestSuite("testQueryPESSIMISTIC_FORCE_INCREMENTLock"));
             suite.addTest(new AdvancedQueryTestSuite("testVersionChangeWithReadLock"));
             suite.addTest(new AdvancedQueryTestSuite("testVersionChangeWithWriteLock"));
+            suite.addTest(new AdvancedQueryTestSuite("testFloatSortWithPessimisticLock"));
             suite.addTest(new AdvancedQueryTestSuite("testNamedQueryAnnotationOverwritePersistenceXML"));
         }
         return suite;
@@ -192,7 +195,8 @@ public void testSetup() {
         employeePopulator.buildExamples();
         //Persist the examples in the database
         employeePopulator.persistExample(session);
-
+        // EntityFloat instances to test issue #2301
+        EntityFloatPopulator.populate(session);
         new RelationshipsTableManager().replaceTables(session);
         //populate the relationships model and persist as well
         new RelationshipsExamples().buildExamples(session);
@@ -2950,4 +2954,26 @@ 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());
+    }
 }