Hi Pinaki, great to see you still around! First I have to thank you for your work on the Criteria stuff. It's a very complex topic but the code is very cleanly written!
The initial reason why I thought we are in trouble is that the CriteriaQuery.toCQL() gets used to create the StoreQuery (_id gets set with the CQL String). I'm still not quite sure why we need the _id but it turned out that the expression handling takes another route though compared to JPQL (different Language) and properly 'walks' down the CriteriaQueryImpl. Reason for drilling into this was the following ticket: https://issues.apache.org/jira/browse/OPENJPA-2668 I'm not quite sure how Olli handles this in SpringData, but if he would properly use cb.parameter then it should work. Otoh a TypedQuery created by em.createQuery(CriteriaQuery) really works a bit different than a normal query. E.g. you cannot 'ask' qry.getParameters(). You will always get an empty set. This seems to be different to Hibernate at least (no clue about EclipseLink). It's really more like using string concatenation - but thankfully without the SQL injection issues I initially was afraid of. LieGrue, strub > On Wednesday, 9 November 2016, 20:30, Pinaki Poddar <[email protected]> > wrote: > > Hi Kevin, > Good to hear from you. How are you doing, Sir? > > My memory of implementation of CriteriaQuery has faded over past 5-6 > years and may not be accurate. > > But going through this email trail the current issue appears somewhat > vague. > > AFAIR, the spec had specified nothing about stringification of a > CriteriaQuery. > > > We, however, presented a stringified form that *approximately* matches an > *equivalent* JPQL query. > The reason was easier tracing or debugging. The goal was not to match > CriteriaQuery.toString() with > a JPQL string simply because such equality can not be established. > > A more precise description of what the expectation is and what is > 'smelly' > (as in a rotten fish?) would help > further analysis. > > > Pinaki > > > On 9 November 2016 at 06:13, Kevin Sutter <[email protected]> wrote: > >> Looking back through the spec and the implementation details, it looks >> like your latter interpretation is accurate. At least from our >> implementation viewpoint. Outside of your experiments earlier, are you >> hearing from customers that they are expecting the alternative >> interpretation? Pinaki was our expert with the Criteria API, so maybe he >> can provide more background. >> >> Thanks, Kevin >> >> On Tue, Nov 8, 2016 at 5:04 PM, Mark Struberg > <[email protected]> >> wrote: >> >>> After digging through the spec I'm not really sure whether this is > even >>> intentional. >>> >>> There are an explicit CriteriaBuilder#parameter methods. And if I use >>> those then I get a perfectly fine >>> "SELECT * FROM Department d WHERE d.name <> :x" >>> >>> Using setParameter("x", "bla") on the resulting > TypedQuery yields perfect >>> results. >>> >>> Happy to get some feedback! >>> >>> LieGrue, >>> strub >>> >>> >>> >>> >>> > On Tuesday, 8 November 2016, 23:30, Mark Struberg >>> <[email protected]> wrote: >>> > > Hi folks! >>> > >>> > >>> > I think I found something smelly in our CriteriaQuery > implementation. >>> This is >>> > related to OPENJPA-2668. >>> > >>> > Currently EntityManager#createQuery(CriteriaQuery cq) returns the > fully >>> > 'interpreted' string. And no parameters. >>> > >>> > >>> > Example: >>> > >>> > CriteriaBuilder cb = em.getCriteriaBuilder(); >>> > CriteriaQuery<Department> q = > cb.createQuery(Department.class); >>> > Root<Department> book = q.from(Department.class); >>> > q.where(cb.notEqual(book.get(Department_.name), "X")); >>> > TypedQuery<Department> dept = em.createQuery(q); >>> > >>> > >>> > This creates the TypedQuery with >>> > >>> > "SELECT * FROM Department d WHERE d.name <> > 'X'" >>> > And no parameter at all. >>> > >>> > But imo it should use >>> > "SELECT * FROM Department d WHERE d.name <> ?" >>> > with dept.getParameters() returning a Parameter with pos 1 and > value >>> > 'X'. >>> > Wdyt? >>> > >>> > >>> > Not quite sure though how it should behave in Expression.In. >>> > >>> > >>> > CriteriaBuilder cb = em.getCriteriaBuilder(); >>> > CriteriaQuery<Customer> cq = > this.cb.createQuery(Customer.class); >>> > Root<Customer> customerRoot = cq.from(Customer.class); >>> > cq.select(customerRoot) >>> > .where(cb.and(cb.equal(custome >>> rRoot.get(Customer_.lastName), >>> > "Hans"), >>> > >>> > customerRoot.get(Customer_.name).in(Arrays.asList("A", > "B", >>> > "C")))); >>> > String queryString = cq.toString(); >>> > >>> > >>> > Currently this returns >>> > >>> > "SELECT c FROM Customer c WHERE c.lastname = 'Hans' > and c.name IN >>> > ('A','B', 'C')" >>> > >>> > This is imo wrong. >>> > But the question is whether it should return >>> > >>> > "SELECT c FROM Customer c WHERE c.lastname = ? and c.name IN > (?)" >>> > or whether it already need to 'extend' the parameters >>> > >>> > "SELECT c FROM Customer c WHERE c.lastname = ? and c.name IN > (?, ?, >>> > ?)" >>> > >>> > Of course the later is what will be used for the prepared > statement. >>> > But the one parameter version might be easier to reuse with > different >>> > parameters? >>> > >>> > LieGrue, >>> > strub >>> > >>> >> >> >
