Fix for JPQL queries where identifier clashes with function

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

@@ -1,6 +1,7 @@
 <!--
 
     Copyright (c) 2018, 2024 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 2024 Contributors to the Eclipse Foundation. 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
@@ -327,6 +328,8 @@
     <persistence-unit name="advanced-jakartadata" transaction-type="RESOURCE_LOCAL">
         <provider>org.eclipse.persistence.jpa.PersistenceProvider</provider>
         <class>org.eclipse.persistence.testing.models.jpa.datatypes.WrapperTypes</class>
+        <class>org.eclipse.persistence.testing.models.jpa.advanced.Room</class>
+        <class>org.eclipse.persistence.testing.models.jpa.advanced.Door</class>
         <exclude-unlisted-classes>true</exclude-unlisted-classes>
         <properties>
             <property name="eclipselink.logging.level" value="${eclipselink.logging.level}"/>

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

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation. 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
@@ -28,6 +29,11 @@
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.util.List;
+import java.util.stream.Stream;
+import org.eclipse.persistence.sessions.server.ServerSession;
+import org.eclipse.persistence.testing.models.jpa.advanced.AdvancedTableCreator;
+import org.eclipse.persistence.testing.models.jpa.advanced.Room;
 
 /**
  * <p>
@@ -45,6 +51,14 @@
  */
 public class JUnitJPQLJakartaDataNoAliasTest extends JUnitTestCase {
     private static final String STRING_DATA = "A String";
+    private static final Room[] ROOMS = new Room[] {
+            null, // Skip array index 0
+            aRoom(1, 1, 1, 1),
+            aRoom(2, 1, 1, 1),
+            aRoom(3, 1, 1, 1),
+            aRoom(4, 1, 1, 1)
+    };
+    private static final long ROOMS_COUNT = ROOMS.length -1; // we ignore the first one with index 0
 
     private static int wrapperId;
 
@@ -86,6 +100,10 @@ public static Test suite() {
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testNoAliasFromWhereAndUPPER"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testGeneratedSelectNoAliasFromWhere"));
         suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testGeneratedSelect"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testUpdateQueryLengthInAssignmentAndExpression"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testSelectQueryLengthInAssignmentAndExpression"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testDeleteQueryLengthInExpressionOnLeft"));
+        suite.addTest(new JUnitJPQLJakartaDataNoAliasTest("testDeleteQueryLengthInExpressionOnRight"));
         return suite;
     }
 
@@ -95,10 +113,13 @@ public static Test suite() {
     public void testSetup() {
         //initialize the global comparer object
         comparer = new JUnitDomainObjectComparer();
+        final ServerSession session = getPersistenceUnitServerSession();
+
         //set the session for the comparer to use
-        comparer.setSession(getPersistenceUnitServerSession());
+        comparer.setSession(session);
 
-        new DataTypesTableCreator().replaceTables(getPersistenceUnitServerSession());
+        new DataTypesTableCreator().replaceTables(session);
+        new AdvancedTableCreator().replaceTables(session);
         clearCache();
         EntityManager em = createEntityManager();
         WrapperTypes wt;
@@ -246,4 +267,63 @@ public void testGeneratedSelectNoAliasFromWhere() {
         WrapperTypes tlWrapperTypes = (WrapperTypes) getPersistenceUnitServerSession().executeQuery(tlQuery);
         Assert.assertTrue("GeneratedSelectNoAliasFromWhere Test Failed", comparer.compareObjects(wrapperTypes, tlWrapperTypes));
     }
+
+    public void testUpdateQueryLengthInAssignmentAndExpression() {
+        resetRooms();
+        long numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                "UPDATE Room SET length = length + 1").executeUpdate());
+        assertTrue("All rooms should be updated", numberOfChanges == ROOMS_COUNT);
+
+        long numberOfRoomsWithLengthChanged = getAllRooms()
+                .filter(room -> room.getLength() == 2)
+                .count();
+        assertTrue("All rooms should have increased length", numberOfRoomsWithLengthChanged == ROOMS_COUNT);
+    }
+
+    public void testSelectQueryLengthInAssignmentAndExpression() {
+        List<Room> roomsWithIdOne  = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                "SELECT this FROM Room WHERE id + length = length + 1", Room.class).getResultList());
+        assertTrue("Number of rooms with ID = 1", roomsWithIdOne.size() == 1);
+    }
+
+    public void testDeleteQueryLengthInExpressionOnLeft() {
+        resetRooms();
+        assertTrue("Number of remaining rooms", getAllRooms().count() == ROOMS_COUNT);
+        int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                "DELETE FROM Room WHERE length = id - 1").executeUpdate());
+        long allRoomsCount = getAllRooms().count();
+        assertTrue("Number of rooms with ID = 1 deleted", numberOfChanges == 1);
+        assertTrue("Number of remaining rooms", allRoomsCount == ROOMS_COUNT - 1);
+    }
+
+    public void testDeleteQueryLengthInExpressionOnRight() {
+        resetRooms();
+        int numberOfChanges = getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                "DELETE FROM Room WHERE id = length + 1").executeUpdate());
+        assertTrue("Number of rooms with ID = 1 deleted", numberOfChanges == 1);
+        assertTrue("Number of remaining rooms", getAllRooms().count() == ROOMS_COUNT - 1);
+    }
+
+    private Stream<Room> getAllRooms() {
+        return getEntityManagerFactory().callInTransaction(em -> em.createQuery(
+                "SELECT r FROM Room r", Room.class).getResultStream());
+    }
+
+    private static Room aRoom(int id, int width, int length, int height) {
+        Room room = new Room();
+        room.setId(id);
+        room.setWidth(width);
+        room.setLength(length);
+        room.setHeight(height);
+        return room;
+    }
+
+    private void resetRooms() {
+        getEntityManagerFactory().runInTransaction(em -> {
+            em.createQuery("DELETE FROM Room").executeUpdate();
+            for (int i = 1; i < ROOMS.length; i++) {
+                em.persist(ROOMS[i]);
+            }
+        });
+    }
 }

jpa/org.eclipse.persistence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/WordParser.java

@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2006, 2022 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2022 IBM Corporation. All rights reserved.
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation. 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
@@ -256,6 +257,7 @@ public int length() {
      */
     public void moveBackward(CharSequence word) {
         cursor -= word.length();
+        wordEndPosition();
     }
 
     /**

jpa/org.eclipse.persistence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractEncapsulatedExpression.java

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2006, 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation. 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
@@ -306,4 +307,19 @@ else if (hasSpaceAfterIdentifier) {
      * like content assist
      */
     protected abstract void toParsedTextEncapsulatedExpression(StringBuilder writer, boolean actual);
+
+    /**
+     * Should be reverted if Jakarta Data mode is enabled and the expression doesn't contain parenthesis.
+     * For example, the query {@code delete from Box where length = 1} contains function name {@code length}.
+     * The function {@code length} expects parameters but there are not parameters in the query.
+     * Then {@code length} is not a function but a state field with an implicit identification variable this, ie.
+     * the query is equivalent to {@code delete from Box where this.length = 1}.
+     *
+     * @return Should be reverted and parsed again as a different expression or should be accepted as parsed expression
+     */
+    @Override
+    protected boolean isInvalid() {
+        return getRoot().isJakartaData() && !hasLeftParenthesis();
+    }
+
 }

jpa/org.eclipse.persistence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractExpression.java

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2006, 2024 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation. 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
@@ -15,6 +16,7 @@
 //
 package org.eclipse.persistence.jpa.jpql.parser;
 
+
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.security.AccessController;
@@ -516,6 +518,22 @@ public final JPQLExpression getRoot() {
         return (parent == null) ? (JPQLExpression) this : parent.getRoot();
     }
 
+    /**
+     * Returns closest nested expression that encapsulates this expression,
+     * or the root expression if not inside a nested expression.
+     *
+     * @return  Parent expression
+     */
+    public final ParentExpression getParentExpression() {
+        if (this instanceof ParentExpression parentExpression) {
+            return parentExpression;
+        } else if (parent == null) {
+            return null;
+        } else {
+            return parent.getParentExpression();
+        }
+    }
+
     /**
      * Returns the encapsulated text of this {@link AbstractExpression}, which can be used in various
      * ways, it can be a keyword, a literal, etc.
@@ -769,6 +787,9 @@ tolerant &&               isParsingComplete(wordParser, word, expression)) {
                     if (factory != null) {
                         child = factory.buildExpression(this, wordParser, word, queryBNF, expression, tolerant);
 
+                        // if an invalid expression came from the factory, ignore it and try fallback
+                        child = revertExpressionIfInvalid(child, wordParser, word);
+
                         if (child != null) {
 
                             // The new expression is a child of the previous expression,
@@ -988,6 +1009,14 @@ else if (currentInfo != null) {
         );
     }
 
+    static AbstractExpression revertExpressionIfInvalid(AbstractExpression expression, WordParser wordParser, String word) {
+        if (expression != null && expression.isInvalid()) {
+            wordParser.moveBackward(word);
+            return null;
+        }
+        return expression;
+    }
+
     /**
      * Right away parses the text by retrieving the {@link ExpressionFactory} for the first word that
      * is extracted from {@link WordParser} at the current location.
@@ -1133,6 +1162,17 @@ public String toParsedText() {
      */
     protected abstract void toParsedText(StringBuilder writer, boolean actual);
 
+    /**
+     * Whether this expression is not valid and should be discarded. If it returns true,
+     * the parser will be reverted to the state before this expression was parsed
+     * and it will attempt to parse a different expression.
+     *
+     * @return True if this expression is invalid and should be discarded, otherwise false. By default returns false, should be overriden if expression should be reverted.
+     */
+    protected boolean isInvalid() {
+        return false;
+    }
+
     @Override
     public final String toString() {
         // toString() should only be called during debugging, thus the cached parsed text

jpa/org.eclipse.persistence.jpa.jpql/src/main/java/org/eclipse/persistence/jpa/jpql/parser/AbstractFromClause.java

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2006, 2024 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation. 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
@@ -343,7 +344,7 @@ else if (hierarchicalQueryClause == null) {
             identificationVariableDeclaration.hasRangeVariableDeclaration() && identificationVariableDeclaration.getRangeVariableDeclaration() instanceof RangeVariableDeclaration rangeVariableDeclaration &&
             rangeVariableDeclaration.hasIdentificationVariable() && rangeVariableDeclaration.getIdentificationVariable() instanceof IdentificationVariable identificationVariable &&
             Expression.THIS.equals(identificationVariable.getText())) {
-            this.getRoot().setGenerateThisPrefix(true);
+            this.getParentExpression().setGenerateThisPrefix(true);
         }
     }