Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JPQL SELECT NEW not joining nor recognising ElementCollection as a collection type #2264

Open
Riva-Tholoor-Philip opened this issue Sep 13, 2024 · 16 comments

Comments

@Riva-Tholoor-Philip
Copy link
Contributor

EclipseLink is rejecting JPQL SELECT NEW clauses where an entity contains an attribute that is an ElementCollection.

For example, SELECT o.comments FROM Rating o WHERE o.id = :id where the column comments is an ElementCollection is correctly returned as a result set.
We can see from the SQL generated by EclipseLink that a JOIN is required to get the data from the Rating_COMMENTS table:

SELECT t1.COMMENTS 
FROM RATING t0 
  LEFT OUTER JOIN Rating_COMMENTS t1 
  ON (t1.Rating_ID = t0.ID) 
WHERE (t0.ID = ?)

However, when we try to use o.comments in a SELECT NEW query like the one below:

SELECT NEW 
  io.openliberty.jpa.data.tests.models.Rating( o.id, o.item, o.numStars, o.reviewer, o.comments )
FROM Rating o 
WHERE o.id = :id

The following error is thrown:

Exception [EclipseLink-0] (Eclipse Persistence Services - 5.0.0.v202408071314-43356e84b79e71022b1656a5462b0a72d70787a4): org.eclipse.persistence.exceptions.JPQLException
Exception Description: Problem compiling [SELECT NEW io.openliberty.jpa.data.tests.models.Rating( o.id, o.item, o.numStars, o.reviewer, o.comments ) FROM Rating o WHERE o.id = :id].
[93, 103] The state field path 'o.comments' cannot be resolved to a collection type. (SELECT NEW io.openliberty.jpa.data.tests.models.Rating(o.id, o.item, o.numStars, o.reviewer, [ o.comments ] ...
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.buildException(HermesParser.java:175)
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.validate(HermesParser.java:364)
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.populateQueryImp(HermesParser.java:298)
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.buildQuery(HermesParser.java:180) 

I would have expected EclipseLink to be able to satisfy this JPQL query with a SQL Query something like:

SELECT t0.ID, t0.NAME, t0.PRICE, t0.NUMSTARS, t0.EMAIL, t0.FIRSTNAME, t0.LASTNAME, t1.COMMENTS 
FROM RATING t0
  LEFT OUTER JOIN Rating_COMMENTS t1 
  ON (t1.Rating_ID = t0.ID) 
WHERE (t0ID = ?)
@rfelcman
Copy link
Contributor

rfelcman commented Dec 3, 2024

Please attach there entities code.

@ajaypaul-ibm
Copy link
Contributor

Hi @rfelcman Please find the Entity mentioned in the issue below

@Entity
public class Rating {

    @Id
    public int id;

    @Embedded
    public Item item;

    public int numStars;

    @Embedded
    public Reviewer reviewer;

    @ElementCollection(fetch = FetchType.EAGER)
    public Set<String> comments;

    // Basic constructor for builder method
    public Rating() {
    }

    // Test constructor to show that SELECT NEW works without comments field
    public Rating(int id, Item item, int stars, Reviewer reviewer) {
        this.id = id;
        this.item = item;
        this.numStars = stars;
        this.reviewer = reviewer;
        this.comments = null;
    }

    // The constructor SELECT NEW should be able to use
    public Rating(int id, Item item, int stars, Reviewer reviewer, Set<String> comments) {
        this.id = id;
        this.item = item;
        this.numStars = stars;
        this.reviewer = reviewer;
        this.comments = comments;
    }

    public static Rating of(int id, Item item, int stars, Reviewer reviewer, String... comments) {
        Rating inst = new Rating();
        inst.id = id;
        inst.item = item;
        inst.numStars = stars;
        inst.reviewer = reviewer;
        inst.comments = Set.of(comments);
        return inst;
    }

    @Embeddable
    public static class Reviewer {
        public String firstName;
        public String lastName;
        public String email;

        public static Reviewer of(String firstName, String lastName, String email) {
            Reviewer inst = new Reviewer();
            inst.firstName = firstName;
            inst.lastName = lastName;
            inst.email = email;
            return inst;
        }
    }

    @Embeddable
    public static class Item {
        public String name;
        public float price;

        public static Item of(String name, float price) {
            Item inst = new Item();
            inst.name = name;
            inst.price = price;
            return inst;
        }

        @Override
        public String toString() {
            return "Item: " + name + " $" + price;
        }
    }
}

@rfelcman
Copy link
Contributor

rfelcman commented Jan 2, 2025

Sorry but, I don't think, that this is bug.
See Jakarta Persistence Specification 3.2 https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a5438

constructor_item ::=
    single_valued_path_expression |
    scalar_expression |
    aggregate_expression |
    identification_variable

I don't think, that collection or array is allowed as constructor_item in any of single_valued_path_expression | scalar_expression | aggregate_expression | identification_variable.

@rfelcman
Copy link
Contributor

rfelcman commented Jan 2, 2025

I think, that this issue is duplicitous with #2193

@ajaypaul-ibm
Copy link
Contributor

Hi @rfelcman We agree with your comment about the specification. But EclipseLink still appears to have an issue, as it does not reject the usage but instead exhibits unexpected behavior by using only the value of the first element in the collection.

@rfelcman
Copy link
Contributor

Hi @rfelcman We agree with your comment about the specification. But EclipseLink still appears to have an issue, as it does not reject the usage but instead exhibits unexpected behavior by using only the value of the first element in the collection.

Sorry, but what do You mean not reject the usage?
There is error/exception message message and application will not continue.

@ajaypaul-ibm
Copy link
Contributor

ajaypaul-ibm commented Jan 14, 2025

Hi @rfelcman
Returning the first element of a collection rather than the whole collection might in some cases lead to a different error being raised due to a mismatch in types. It won't always though.  When you define the record in a way that there isn't a type mismatch, then you get the wrong value. That is a bug.
If you want to see a scenario where it returns wrong data,
Consider this entity


public record RomanNumeral(
                String name,
                String romanNumeral,
                Object romanNumeralSymbols

) {


    @Override
    public String toString() {
        StringBuilder s = new StringBuilder();
        s.append(name).append(" ");
        s.append(romanNumeral).append(" ( ");
        s.append(romanNumeralSymbols).append(' ');
        s.append(")");
        return s.toString();
    }
}


And the test :

@Test
   public void testQueryWithRecordResult() {
       assertEquals(List.of("eleven XI ( X I )",
                            "five V ( V )",
                            "nineteen XIX ( X I X )",
                            "seven VII ( V I I )",
                            "seventeen XVII ( X V I I )",
                            "thirteen XIII ( X I I I )",
                            "three III ( I I I )",
                            "two II ( I I )"),
                    primes.romanNumeralsLessThanEq(20)
                                    .stream()
                                    .map(RomanNumeral::toString)
                                    .collect(Collectors.toList()));
   }

When you run the tests, testQueryWithRecordResult of course will fail, but the failure message reveals that EclipseLink is returning a value for the ArrayList attribute, but it isn't a collection of String. It is a single string, seemingly a randomly chosen element of the collection.  That's a bug.
EclipseLink either needs to raise an error indicating it won't support this, or else return the correct value for the Collection attribute. Returning incorrect data is a bug.

@rfelcman
Copy link
Contributor

Sorry I confused about last provided example. I don't see there any JPA or EclipseLink related code.

@ajaypaul-ibm
Copy link
Contributor

@rfelcman Can you try to use the above entity (RomanNumeral) and try to execute query using NEW keyword?

@rfelcman
Copy link
Contributor

rfelcman commented Jan 14, 2025

There is my test case which works well see
com.oracle.jpa.bugtest.TestRecord#queryNEWTest
DB is Derby, table (TEST_TAB_RECORD) creation script is init.sql.
Please check it modify and pass back.

Can you try to use the above entity (RomanNumeral) and try to execute query using NEW keyword?

RomanNumeral is record and can't be Jakarta Persistence @Entity.
JakartaPersistence32POC.tar.gz

@ajaypaul-ibm
Copy link
Contributor

Hi @rfelcman Can you try this modified one JakartaPersistence32POC_modified.zip . New test called queryNEWTest is added in this.

@ajaypaul-ibm
Copy link
Contributor

@rfelcman
Copy link
Contributor

But if You are mean

// Simulate the query for Roman numerals less than or equal to 20
List<RomanNumeral> result = em
                    .createQuery("SELECT r FROM RomanNumeral r WHERE r.value <= :maxValue", RomanNumeral.class)
                    .setParameter("maxValue", 20)
                    .getResultList();

from modified tests where

package com.oracle.jpa.bugtest;

public record RomanNumeral(
        String name,
        String romanNumeral,
        Object romanNumeralSymbols) {
...

it's incorrect as record RomanNumeral can't be used as an entity see
specification 2.1. The Entity Class
https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#a18

@ajaypaul-ibm
Copy link
Contributor

ajaypaul-ibm commented Jan 14, 2025

Hi @rfelcman You are right about the records. I was refering to Jakarta Data test. Records can be Jakarta Data entities, in which case Jakarta Data creates a separate JPA entity class that is not a record but is based on the record
Can you try to test using below details (This will work with JPA):
Prime Entity :

@Entity
@Table(name = "Prime")
public class Prime {
    @Id
    @Column(nullable = false)
    private int numberId;

    private String name;

    private String romanNumeral;

    @ElementCollection(fetch = FetchType.EAGER)
    @CollectionTable(name = "Prime_ROMANNUMERALSYMBOLS", joinColumns = @JoinColumn(name = "Prime_NUMBERID"))
    private List<String> romanNumeralSymbols;

    private String binaryDigits;

    @Column(nullable = false)
    private boolean even;

    @Column(nullable = false)
    private int sumOfBits;

    // Getters, setters, and constructors
}

Create a DTO like below :

public class RomanNumeral {
    private String name;
    private String romanNumeral;
    private List<String> romanNumeralSymbols;

    public RomanNumeral(String name, String romanNumeral, List<String> romanNumeralSymbols) {
        this.name = name;
        this.romanNumeral = romanNumeral;
        this.romanNumeralSymbols = romanNumeralSymbols;
    }

    // Getters and setters
}

JPQL Query :

TypedQuery<RomanNumeral> query = entityManager.createQuery(
    "SELECT NEW com.example.RomanNumeral(p.name, p.romanNumeral, p.romanNumeralSymbols) " +
    "FROM Prime p WHERE p.numberId <= :numberId ORDER BY p.name", RomanNumeral.class);
query.setParameter("numberId", 20);
List<RomanNumeral> results = query.getResultList();

Test case :

@Test
public void testRomanNumeralsQuery() {
    EntityManager em = entityManagerFactory.createEntityManager();
    em.getTransaction().begin();

    // Insert sample data into the Prime table
    Prime prime = new Prime(1, "Three", "III", List.of("I", "I", "I"), "11", false, 2);
    em.persist(prime);

    em.getTransaction().commit();

    TypedQuery<RomanNumeral> query = em.createQuery(
        "SELECT NEW com.example.RomanNumeral(p.name, p.romanNumeral, p.romanNumeralSymbols) " +
        "FROM Prime p WHERE p.numberId <= :numberId ORDER BY p.name", RomanNumeral.class);
    query.setParameter("numberId", 1);
    List<RomanNumeral> results = query.getResultList();

    assertEquals(1, results.size());
    assertEquals("Three", results.get(0).getName());
}

@rfelcman
Copy link
Contributor

But as I mentioned above array or collection is not allowed by Jakarta Persistence spec
#2264 (comment)
.......p.romanNumeralSymbols) " +
private List<String> romanNumeralSymbols;

@njr-11
Copy link
Contributor

njr-11 commented Jan 14, 2025

I just read through this issue and there are a lot of mistakes in representing the Jakarta Data scenario, including some in the latest attempt.

@ajaypaul-ibm please make sure you can run the test yourself before posting it for Radek to look at. You should have it log the output of the query to System.out and be able to post an example of what the corrupted output looks like when running the test. For purposes of demonstrating the unexpected behavior, I would get rid of the asserts. They obscure what is actually being returned. Later if you want an automated test, you can put them back in then.
Also, the Jakarta Data scenario has Object romanNumeralSymbols under the RomanNumeral record, not what you used.

But as I mentioned above array or collection is not allowed by Jakarta Persistence spec

Definitely. The problem is that EclipseLink is NOT disallowing the collection attribute and instead successfully runs the query and returns wrong data. That is what Ajay is attempting to demonstrate. Thanks for all of your patience here. I think he is getting close to doing so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants