jdaugherty commented on code in PR #16816:
URL: https://github.com/apache/grails-core/pull/16816#discussion_r4474602121
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java:
##########
@@ -19,68 +19,51 @@
package org.grails.orm.hibernate.query;
import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
import java.util.List;
-import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.ServiceLoader;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.stream.Collectors;
import jakarta.persistence.criteria.AbstractQuery;
import jakarta.persistence.criteria.CriteriaBuilder;
import jakarta.persistence.criteria.Expression;
import jakarta.persistence.criteria.From;
-import jakarta.persistence.criteria.Join;
-import jakarta.persistence.criteria.JoinType;
-import jakarta.persistence.criteria.ParameterExpression;
import jakarta.persistence.criteria.Path;
import jakarta.persistence.criteria.Predicate;
-import jakarta.persistence.criteria.Subquery;
import org.hibernate.query.criteria.HibernateCriteriaBuilder;
-import org.hibernate.query.criteria.JpaSubQuery;
import org.springframework.core.convert.ConversionService;
-import grails.gorm.DetachedCriteria;
import org.grails.datastore.gorm.query.criteria.DetachedAssociationCriteria;
import org.grails.datastore.mapping.core.exceptions.ConfigurationException;
import org.grails.datastore.mapping.model.PersistentEntity;
import org.grails.datastore.mapping.model.PersistentProperty;
import org.grails.datastore.mapping.model.types.Association;
-import org.grails.datastore.mapping.query.Projections;
import org.grails.datastore.mapping.query.Query;
-import org.grails.datastore.mapping.query.api.QueryableCriteria;
+import org.grails.datastore.mapping.query.criteria.FunctionCallingCriterion;
import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity;
-import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernatePersistentProperty;
-import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateToManyProperty;
/**
- * A class that generates predicates for a given list of criteria.
+ * Converts Grails Datastore query criteria into JPA {@link Predicate}
instances.
+ * Dispatches criteria to dedicated {@link InternalCriterionHandler}
implementations
+ * while supporting custom criterion extension via {@link CriterionHandler}.
*
- * @since 8.0
+ * @author Graeme Rocher
+ * @since 6.0
Review Comment:
`PredicateGenerator` was added for Hibernate 7 (52e87b8c4b), and 841f6fbfaa
set `@since 8.0` on it on purpose. Please keep that rather than `@author Graeme
Rocher` / `@since 6.0`.
The rewrite also drops the javadoc of the public SPI, which is the only
documentation it has on 8.0.x (the guide does not cover it):
- `CriterionHandler`: registered handlers run before the built-in handling
and can override it. The example that is left no longer compiles, since `c` is
undefined without the `def c = criterion as MyCustomCriterion` line.
- `CriterionHandlerProvider`: the
`META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$CriterionHandlerProvider`
file name and the example.
- `registerCriterionHandler`: the type has to match exactly, and a
programmatic registration wins over a `ServiceLoader` one.
- `clearCustomCriterionHandlers`.
The comments that explain the moved code are gone as well:
`convertComparisonValue` (why a plain value is not wrapped in a literal, which
is what keeps comparisons on a `UserType` property working), `asExpression`,
`handleEqualsIgnoreCase`, `aliasColumn` and `nativeSqlArguments`. Please carry
them over to where the code lives now.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/query/jpa/JpaQueryBuilder.java:
##########
@@ -605,6 +606,73 @@ public int handle(PersistentEntity entity, Query.Criterion
criterion, StringBuil
}
});
+ queryHandlers.put(FunctionCallingCriterion.class, new QueryHandler() {
+ public int handle(PersistentEntity entity, Query.Criterion
criterion, StringBuilder q, StringBuilder whereClause, String logicalName, int
position, List parameters, ConversionService conversionService, boolean
allowJoins, boolean hibernateCompatible) {
+ FunctionCallingCriterion functionCriterion =
(FunctionCallingCriterion) criterion;
+ String name = functionCriterion.getProperty();
+ validateProperty(entity, name, FunctionCallingCriterion.class);
+ if (functionCriterion.isOnValue()) {
+ throw new InvalidDataAccessResourceUsageException("A
function on the value of a comparison is not supported: " +
functionCriterion.getFunctionName());
+ }
+ Query.PropertyCriterion wrapped =
functionCriterion.getPropertyCriterion();
+ if (wrapped instanceof Query.ILike) {
+ whereClause.append("lower(")
+ .append(functionCriterion.getFunctionName())
+ .append(OPEN_BRACKET)
+ .append(logicalName)
+ .append(DOT)
+ .append(name)
+ .append(CLOSE_BRACKET)
+ .append(") like lower(")
+ .append(PARAMETER_PREFIX)
+ .append(++position)
+ .append(CLOSE_BRACKET);
+ parameters.add(wrapped.getValue());
+ return position;
+ }
+ whereClause.append(functionCriterion.getFunctionName())
Review Comment:
The property goes through `validateProperty`, but the function name is
appended to the query as it is. On Hibernate 5, `list()` looks the name up in
the dialect and rejects one it does not know (`Unsupported function defined in
query [x]`). Here a `FunctionCallingCriterion` whose function name is not a
plain identifier changes the `WHERE` clause: in a probe it made `updateAll`
update every row, on Hibernate 5 and on Hibernate 7. The same holds for
`list()` on Hibernate 7, where the `default` branch of
`FunctionCallingCriterionHandler.applyFunction` hands any name to
`criteriaBuilder.function`.
That branch also types every other function as `Integer`, so a function that
returns a string fails with a `ConversionFailedException` on the value it is
compared with.
Please accept only the ten functions the where DSL supports
(`SUPPORTED_FUNCTIONS` in `DetachedCriteriaTransformer`) in both places, or at
least require a plain identifier, and throw
`InvalidDataAccessResourceUsageException` for anything else, as Hibernate 5
does.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/InternalCriterionHandler.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.grails.orm.hibernate.query;
+
+import jakarta.persistence.criteria.Predicate;
+
+import org.grails.datastore.mapping.query.Query;
+
+/**
+ * Internal strategy interface for converting a specific {@link
Query.QueryElement}
+ * into a JPA {@link Predicate} using the provided {@link CriterionContext}.
+ *
+ * @param <T> the query element type
+ * @since 8.0.1
+ */
+@FunctionalInterface
+public interface InternalCriterionHandler<T extends Query.QueryElement> {
Review Comment:
`InternalCriterionHandler`, `CriterionContext` and the ten handler classes
are public types with public constructors and static methods, new in a patch
release. Nothing outside `org.grails.orm.hibernate.query` uses them (the specs
go through `PredicateGenerator`), so they can be package-private, as
`QueryAliasRegistrar` is. That keeps them free to change, and it avoids a
second public context type next to `QueryPathContext`, which 8.1.x already
hands to custom handlers.
Optional: `findHandler` walks the superclass chain, so registering
`PropertyCriterion`, `SubqueryCriterion`, `PropertyNameCriterion`,
`PropertyComparisonCriterion` and `Junction` covers their subclasses. The 42
entries for the subclasses each create another handler instance and change
nothing.
##########
grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/query/jpa/JpaQueryBuilderFunctionSpec.groovy:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.grails.datastore.mapping.query.jpa
+
+import jakarta.persistence.Entity
+import jakarta.persistence.Id
+import
org.grails.datastore.mapping.keyvalue.mapping.config.KeyValueMappingContext
+import org.grails.datastore.mapping.model.PersistentEntity
+import org.grails.datastore.mapping.query.Projections
+import org.grails.datastore.mapping.query.Query
+import org.grails.datastore.mapping.query.api.QueryableCriteria
+import org.grails.datastore.mapping.query.criteria.FunctionCallingCriterion
+import org.springframework.dao.InvalidDataAccessResourceUsageException
+import spock.lang.Specification
+import spock.lang.Unroll
+
+class JpaQueryBuilderFunctionSpec extends Specification {
+
+ PersistentEntity entity
+
+ def setup() {
+ def context = new KeyValueMappingContext('test')
+ entity = context.addPersistentEntity(JpaFunctionTestPerson)
+ }
+
+ @Unroll
+ void "FunctionCallingCriterion renders #expectedSql for
#criterion.class.simpleName"() {
+ given:
+ def criteria = [new FunctionCallingCriterion(functionName, criterion)]
+
+ when:
+ def info = new JpaQueryBuilder(entity, criteria).buildDelete()
+
+ then:
+ info.query == "DELETE FROM ${JpaFunctionTestPerson.name}
jpaFunctionTestPerson WHERE (${expectedSql})"
+ info.parameters == expectedParams
+
+ where:
+ functionName | criterion ||
expectedSql | expectedParams
+ 'upper' | new Query.Equals('name', 'BOB') ||
'upper(jpaFunctionTestPerson.name) = :p1' | ['BOB']
+ 'upper' | new Query.NotEquals('name', 'BOB') ||
'upper(jpaFunctionTestPerson.name) != :p1' | ['BOB']
+ 'year' | new Query.GreaterThan('age', 2000) ||
'year(jpaFunctionTestPerson.age) > :p1' | [2000]
+ 'year' | new Query.GreaterThanEquals('age', 2000) ||
'year(jpaFunctionTestPerson.age) >= :p1' | [2000]
+ 'year' | new Query.LessThan('age', 2000) ||
'year(jpaFunctionTestPerson.age) < :p1' | [2000]
+ 'year' | new Query.LessThanEquals('age', 2000) ||
'year(jpaFunctionTestPerson.age) <= :p1' | [2000]
+ 'lower' | new Query.Like('name', 'bob%') ||
'lower(jpaFunctionTestPerson.name) LIKE :p1' | ['bob%']
+ 'upper' | new Query.ILike('name', 'bob%') ||
'lower(upper(jpaFunctionTestPerson.name)) like lower(:p1)' | ['bob%']
+ 'upper' | new Query.Equals('name', null) ||
'upper(jpaFunctionTestPerson.name) IS NULL' | []
+ 'upper' | new Query.NotEquals('name', null) ||
'upper(jpaFunctionTestPerson.name) IS NOT NULL' | []
+ 'upper' | new Query.In('name', ['A']) ||
'upper(jpaFunctionTestPerson.name) IN (:p1)' | ['A']
+ 'upper' | new Query.In('name', ['A', 'B', 'C']) ||
'upper(jpaFunctionTestPerson.name) IN (:p1,:p2,:p3)' | ['A', 'B', 'C']
+ }
+
+ void "FunctionCallingCriterion with In subquery renders subselect and
advances parameter positions"() {
+ given:
+ QueryableCriteria subquery = Stub(QueryableCriteria) {
+ getPersistentEntity() >> entity
+ getProjections() >> [Projections.property('name')]
+ getCriteria() >> [new Query.Equals('name', 'Alice'), new
Query.GreaterThan('age', 21)]
+ }
+ def criteria = [
+ new FunctionCallingCriterion('upper', new Query.In('name',
subquery)),
+ new FunctionCallingCriterion('lower', new Query.Equals('name',
'bob'))
+ ]
+
+ when:
+ def info = new JpaQueryBuilder(entity, criteria).buildUpdate(age: 30)
+
+ then:
+ info.query == "UPDATE ${JpaFunctionTestPerson.name}
jpaFunctionTestPerson SET jpaFunctionTestPerson.age=:p1 WHERE
(upper(jpaFunctionTestPerson.name) IN (SELECT jpaFunctionTestPerson1.name FROM
${JpaFunctionTestPerson.name} jpaFunctionTestPerson1 WHERE
jpaFunctionTestPerson1.name=:p2jpaFunctionTestPerson1.age > :p3) AND
lower(jpaFunctionTestPerson.name) = :p4)"
Review Comment:
The expected query has no `AND` between the two criteria of the subquery
(`...name=:p2jpaFunctionTestPerson1.age > :p3`), so this pins a query that
Hibernate cannot parse. Executed at this head, `inList('name', sub)` with two
criteria in `sub`, followed by `updateAll`, throws a `SyntaxException` on
Hibernate 7 and a `QuerySyntaxException` on Hibernate 5, and a subquery without
criteria renders `WHERE )`.
`buildSubQuery` appends the criteria with nothing between them. This PR
already changes that loop to carry `position`, so please join them here as
well: #16822 (e036996491) does it with `buildWhereClauseForCriterion` and a
`Query.Conjunction`, and leaves `WHERE` out for an empty list. Then this
expectation can be a query that runs. If that change stays in #16822, give this
stub a single criterion so the spec does not assert broken output until then.
##########
grails-test-examples/hibernate7/grails-hibernate/src/integration-test/groovy/functional/tests/CriteriaComparisonScenarioSpec.groovy:
##########
@@ -0,0 +1,366 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package functional.tests
+
+import grails.gorm.transactions.Rollback
+import grails.testing.mixin.integration.Integration
+import spock.lang.Specification
+import spock.lang.Unroll
+
+/**
+ * Runs each comparison and null criterion against a real database through the
three public
+ * query surfaces (where, the criteria builder and dynamic finders) and checks
the returned rows,
+ * not the shape of the predicate that was built.
+ */
+@Integration(applicationClass = Application)
+@Rollback
+class CriteriaComparisonScenarioSpec extends Specification {
+
+ private void seed() {
+ def ann = new ScenarioOwner(name: 'ann').save(flush: true,
failOnError: true)
+ def bob = new ScenarioOwner(name: 'bob').save(flush: true,
failOnError: true)
+ new ScenarioItem(name: 'a', qty: 1, status: ScenarioStatus.NEW, owner:
ann, dims: new ScenarioDims(width: 10)).save(flush: true, failOnError: true)
+ new ScenarioItem(name: 'b', qty: 2, status: ScenarioStatus.NEW, owner:
ann, dims: new ScenarioDims(width: 20)).save(flush: true, failOnError: true)
+ new ScenarioItem(name: 'c', qty: 3, status: ScenarioStatus.DONE,
owner: bob, dims: new ScenarioDims(width: 30)).save(flush: true, failOnError:
true)
+ new ScenarioItem(name: 'd', qty: null, status: ScenarioStatus.DONE,
owner: null, dims: null).save(flush: true, failOnError: true)
+ ScenarioItem.withSession { it.clear() }
+ }
+
+ private static List<String> names(Collection<ScenarioItem> items) {
+ items*.name.sort()
+ }
+
+ void "where: simple eq"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty == 2 }.list()
+
+ then:
+ names(rows) == ['b']
+ }
+
+ void "where: != leaves out rows whose value is null, as SQL does"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty != 2 }.list()
+
+ then:
+ names(rows) == ['a', 'c']
+ }
+
+ void "where: simple gt"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty > 1 }.list()
+
+ then:
+ names(rows) == ['b', 'c']
+ }
+
+ void "where: simple ge"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty >= 2 }.list()
+
+ then:
+ names(rows) == ['b', 'c']
+ }
+
+ void "where: simple lt"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty < 3 }.list()
+
+ then:
+ names(rows) == ['a', 'b']
+ }
+
+ void "where: simple le"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty <= 2 }.list()
+
+ then:
+ names(rows) == ['a', 'b']
+ }
+
+ void "where: simple is null"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty == null }.list()
+
+ then:
+ names(rows) == ['d']
+ }
+
+ void "where: simple is not null"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty != null }.list()
+
+ then:
+ names(rows) == ['a', 'b', 'c']
+ }
+
+ void "where: simple in list"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty in [1, 3] }.list()
+
+ then:
+ names(rows) == ['a', 'c']
+ }
+
+ void "where: simple and"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty > 1 && name != 'c' }.list()
+
+ then:
+ names(rows) == ['b']
+ }
+
+ void "where: simple or"() {
+ given:
+ seed()
+
+ when:
+ def rows = ScenarioItem.where { qty == 1 || qty == 3 }.list()
+
+ then:
+ names(rows) == ['a', 'c']
+ }
+
+ void "where: negation follows SQL three-valued logic, unlike != which
includes null rows"() {
Review Comment:
The name still says `!=` includes null rows. The Hibernate 5 copy has the
same line.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/query/jpa/JpaQueryBuilder.java:
##########
@@ -961,4 +1031,29 @@ else if (criterion instanceof AssociationCriteria) {
return position;
}
+
+ private static String functionOperator(Query.PropertyCriterion wrapped) {
+ if (wrapped instanceof Query.Equals) {
+ return " = ";
+ }
+ if (wrapped instanceof Query.NotEquals) {
+ return " != ";
+ }
+ if (wrapped instanceof Query.GreaterThan) {
+ return " > ";
+ }
+ if (wrapped instanceof Query.GreaterThanEquals) {
+ return " >= ";
+ }
+ if (wrapped instanceof Query.LessThan) {
+ return " < ";
+ }
+ if (wrapped instanceof Query.LessThanEquals) {
+ return " <= ";
+ }
+ if (wrapped instanceof Query.Like) {
Review Comment:
`RLike` extends `Like`, so a function over an `rlike` is rendered as `LIKE`
in a bulk query: `lower(name)` with `rlike '^r.x$'` matches a row through
`list()` and updates none through `updateAll`. A plain `RLike` is rejected
there as unsupported; returning `null` for it here does the same.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PropertyCriterionHandler.java:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.grails.orm.hibernate.query;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Collectors;
+
+import jakarta.persistence.criteria.Expression;
+import jakarta.persistence.criteria.Predicate;
+import jakarta.persistence.criteria.Subquery;
+
+import org.hibernate.query.criteria.HibernateCriteriaBuilder;
+import org.hibernate.query.criteria.JpaSubQuery;
+
+import grails.gorm.DetachedCriteria;
+import org.grails.datastore.mapping.core.exceptions.ConfigurationException;
+import org.grails.datastore.mapping.model.PersistentProperty;
+import org.grails.datastore.mapping.query.Projections;
+import org.grails.datastore.mapping.query.Query;
+import org.grails.datastore.mapping.query.api.QueryableCriteria;
+import
org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity;
+
+/**
+ * Handles criteria targeting a persistent property, such as equality,
relational comparisons,
+ * pattern matching, collection size, and IN/BETWEEN checks.
+ *
+ * @since 8.0.1
+ */
+public class PropertyCriterionHandler implements
InternalCriterionHandler<Query.PropertyCriterion> {
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public Predicate handle(CriterionContext context, Query.PropertyCriterion
pc) {
+ String propertyName = pc.getProperty();
+ Expression<?> propertyPath = context.resolvePath(propertyName);
+ if (propertyPath == null) {
+ String className = context.getEntity() != null ?
context.getEntity().getJavaClass().getName() : "unknown";
+ throw new ConfigurationException("Cannot use comparison criteria
on non-existent property [" + propertyName + "] of class [" + className + "]");
+ }
+
+ HibernateCriteriaBuilder criteriaBuilder =
context.getCriteriaBuilder();
+
+ if (pc instanceof EqualsIgnoreCase) {
+ return handleEqualsIgnoreCase(context, pc, propertyPath);
+ } else if (pc instanceof Query.Equals) {
+ return handleEquals(context, pc, propertyPath);
+ } else if (pc instanceof Query.NotEquals) {
+ return handleNotEquals(context, pc, propertyPath);
+ } else if (pc instanceof Query.ILike) {
+ return criteriaBuilder.ilike((Expression<String>) propertyPath,
(String) context.convertValue(propertyName, pc.getValue(), propertyPath));
+ } else if (pc instanceof Query.RLike rLike) {
+ return handleRLike(criteriaBuilder, (Expression<String>)
propertyPath, rLike);
+ } else if (pc instanceof Query.Like) {
+ return criteriaBuilder.like((Expression<String>) propertyPath,
(String) context.convertValue(propertyName, pc.getValue(), propertyPath));
+ } else if (pc instanceof Query.GreaterThan) {
+ Object value = context.convertComparisonValue(propertyName,
pc.getValue(), propertyPath);
+ return value instanceof Comparable<?> comparable ?
+ criteriaBuilder.greaterThan((Expression<? extends Comparable>)
propertyPath, (Comparable) comparable) :
+ criteriaBuilder.greaterThan((Expression<? extends Comparable>)
propertyPath, (Expression) context.asExpression(value, propertyPath));
+ } else if (pc instanceof Query.GreaterThanEquals) {
+ Object value = context.convertComparisonValue(propertyName,
pc.getValue(), propertyPath);
+ return value instanceof Comparable<?> comparable ?
+ criteriaBuilder.greaterThanOrEqualTo((Expression<? extends
Comparable>) propertyPath, (Comparable) comparable) :
+ criteriaBuilder.greaterThanOrEqualTo((Expression<? extends
Comparable>) propertyPath, (Expression) context.asExpression(value,
propertyPath));
+ } else if (pc instanceof Query.LessThan) {
+ Object value = context.convertComparisonValue(propertyName,
pc.getValue(), propertyPath);
+ return value instanceof Comparable<?> comparable ?
+ criteriaBuilder.lessThan((Expression<? extends Comparable>)
propertyPath, (Comparable) comparable) :
+ criteriaBuilder.lessThan((Expression<? extends Comparable>)
propertyPath, (Expression) context.asExpression(value, propertyPath));
+ } else if (pc instanceof Query.LessThanEquals) {
+ Object value = context.convertComparisonValue(propertyName,
pc.getValue(), propertyPath);
+ return value instanceof Comparable<?> comparable ?
+ criteriaBuilder.lessThanOrEqualTo((Expression<? extends
Comparable>) propertyPath, (Comparable) comparable) :
+ criteriaBuilder.lessThanOrEqualTo((Expression<? extends
Comparable>) propertyPath, (Expression) context.asExpression(value,
propertyPath));
+ } else if (pc instanceof Query.In) {
+ return handleIn(context, pc, propertyPath, propertyName);
+ } else if (pc instanceof Query.Between between) {
+ return criteriaBuilder.between((Expression<? extends Comparable>)
propertyPath,
+ (Comparable) context.convertValue(propertyName,
between.getFrom(), propertyPath),
+ (Comparable) context.convertValue(propertyName,
between.getTo(), propertyPath));
+ } else if (pc instanceof Query.SizeEquals) {
+ return
criteriaBuilder.equal(criteriaBuilder.size(context.resolveCollectionPath(propertyPath)),
(Integer) pc.getValue());
+ } else if (pc instanceof Query.SizeNotEquals) {
+ return
criteriaBuilder.notEqual(criteriaBuilder.size(context.resolveCollectionPath(propertyPath)),
(Integer) pc.getValue());
+ } else if (pc instanceof Query.SizeGreaterThan) {
+ return
criteriaBuilder.greaterThan(criteriaBuilder.size(context.resolveCollectionPath(propertyPath)),
(Integer) pc.getValue());
+ } else if (pc instanceof Query.SizeGreaterThanEquals) {
+ return
criteriaBuilder.greaterThanOrEqualTo(criteriaBuilder.size(context.resolveCollectionPath(propertyPath)),
(Integer) pc.getValue());
+ } else if (pc instanceof Query.SizeLessThan) {
+ return
criteriaBuilder.lessThan(criteriaBuilder.size(context.resolveCollectionPath(propertyPath)),
(Integer) pc.getValue());
+ } else if (pc instanceof Query.SizeLessThanEquals) {
+ return
criteriaBuilder.lessThanOrEqualTo(criteriaBuilder.size(context.resolveCollectionPath(propertyPath)),
(Integer) pc.getValue());
+ }
+
+ throw new UnsupportedOperationException("Unsupported criterion: " +
pc.getClass().getName());
+ }
+
+ @SuppressWarnings("unchecked")
+ private static Predicate handleEquals(CriterionContext context,
Query.PropertyCriterion pc, Expression<?> propertyPath) {
+ if (pc.getValue() instanceof QueryableCriteria qc) {
+ Subquery<?> subquery = buildSubquery(context, propertyPath, qc,
pc.getProperty());
+ return context.getCriteriaBuilder().equal(propertyPath, subquery);
+ } else {
+ return context.getCriteriaBuilder().equal(propertyPath,
context.convertComparisonValue(pc.getProperty(), pc.getValue(), propertyPath));
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private static Predicate handleEqualsIgnoreCase(CriterionContext context,
Query.PropertyCriterion pc, Expression<?> propertyPath) {
+ Object value = pc.getValue();
+ if (!String.class.equals(propertyPath.getJavaType()) || !(value
instanceof CharSequence)) {
+ return handleEquals(context, pc, propertyPath);
+ }
+ return context.getCriteriaBuilder().equal(
+ context.getCriteriaBuilder().lower((Expression<String>)
propertyPath),
+ value.toString().toLowerCase(Locale.ROOT));
+ }
+
+ @SuppressWarnings("unchecked")
+ private static Predicate handleNotEquals(CriterionContext context,
Query.PropertyCriterion pc, Expression<?> propertyPath) {
+ Object value = pc.getValue();
+ HibernateCriteriaBuilder criteriaBuilder =
context.getCriteriaBuilder();
+ if (value == null) {
+ return criteriaBuilder.isNotNull(propertyPath);
+ }
+ if (value instanceof QueryableCriteria qc) {
+ Subquery<?> subquery = buildSubquery(context, propertyPath, qc,
pc.getProperty());
+ return criteriaBuilder.notEqual(propertyPath, subquery);
+ }
+ return criteriaBuilder.notEqual(propertyPath,
context.convertComparisonValue(pc.getProperty(), value, propertyPath));
Review Comment:
On the question for the team: I agree with leaving the null rows out. At
8803d54c23 the same where query picked different rows depending on the
operation. With `qty` null on one row, `where { qty != 2 }.list()`, `count()`,
`ne 'qty', 2` and `findAllByQtyNotEqual(2)` returned that row, while `where {
qty != 2 }.updateAll(..)` and `deleteAll()` left it alone (`JpaQueryBuilder`
renders a plain `!=`), and so did `!(qty == 2)`, `not { eq 'qty', 2 }`, `!(qty
in [2])` and `findAllByQtyNotInList([2])`. With this commit they all agree.
It does change the rows an application on 8.0.0 gets back from `!=`, `ne`
and the `NotEqual` finders, also inside an `||`. Please add a section to
`upgrading80x.adoc` ("As of Grails 8.0.1 ...") that says so and shows how to
keep the null rows (`qty != 2 || qty == null`), and a sentence next to `!=` /
`ne` in the Hibernate 7 `whereQueries.adoc` and criteria docs.
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/detachedcriteria/WhereQueryFunctionsSpec.groovy:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package grails.gorm.tests.detachedcriteria
+
+import grails.gorm.DetachedCriteria
+import grails.gorm.annotation.Entity
+import grails.gorm.hibernate.HibernateEntity
+import grails.gorm.tests.HibernateGormDatastoreSpec
+import org.grails.datastore.mapping.query.Query
+import org.grails.datastore.mapping.query.criteria.FunctionCallingCriterion
+
+class WhereQueryFunctionsSpec extends HibernateGormDatastoreSpec {
+
+ def setupSpec() {
+ manager.registerDomainClasses(WhereQueryFunctionEntity)
+ }
+
+ private void seedEntities() {
+ new WhereQueryFunctionEntity(name: 'a', label: 'Alpha', amount: 1,
created: date(2011, 1, 1)).save(flush: true)
+ new WhereQueryFunctionEntity(name: 'b', label: 'Beta', amount: 2,
created: date(2012, 1, 1)).save(flush: true)
+ new WhereQueryFunctionEntity(name: 'c', label: null, amount: 3,
created: date(2013, 1, 1)).save(flush: true)
+ new WhereQueryFunctionEntity(name: 'd', label: 'Delta', amount: null,
created: null).save(flush: true)
+ }
+
+ private static Date date(int year, int month, int day) {
+ new GregorianCalendar(year, month - 1, day).time
+ }
+
+ void "the documented case-insensitive and upper/lower functions execute"()
{
+ given:
+ seedEntities()
+
+ when:
+ def upperMatches = WhereQueryFunctionEntity.where { upper(name) == 'A'
}.list()*.name
+ def lowerMatches = WhereQueryFunctionEntity.where { lower(name) == 'a'
}.list()*.name
+ def upperLike = WhereQueryFunctionEntity.where { upper(name) ==~ 'A%'
}.list()*.name
+ def upperIn = WhereQueryFunctionEntity.where { upper(name) in ['A',
'B'] }.list()*.name.sort()
+
+ then:
+ upperMatches == ['a']
+ lowerMatches == ['a']
+ upperLike == ['a']
+ upperIn == ['a', 'b']
+ }
+
+ void "the documented date functions execute"() {
+ given:
+ seedEntities()
+
+ when:
+ def bornIn2011 = WhereQueryFunctionEntity.where { year(created) ==
2011 }.list()*.name
+ def bornAfter2011 = WhereQueryFunctionEntity.where { year(created) >
2011 }.list()*.name.sort()
+ def bornFrom2012 = WhereQueryFunctionEntity.where { year(created) >=
2012 }.list()*.name.sort()
+ def bornBefore2013 = WhereQueryFunctionEntity.where { year(created) <
2013 }.list()*.name.sort()
+ def bornUpTo2012 = WhereQueryFunctionEntity.where { year(created) <=
2012 }.list()*.name.sort()
+
+ then:
+ bornIn2011 == ['a']
+ bornAfter2011 == ['b', 'c']
+ bornFrom2012 == ['b', 'c']
+ bornBefore2013 == ['a', 'b']
+ bornUpTo2012 == ['a', 'b']
+ }
+
+ void "the documented string functions execute"() {
+ given:
+ seedEntities()
+
+ when:
+ def trimmed = WhereQueryFunctionEntity.where { trim(name) == 'a'
}.list()*.name
+ def oneCharacter = WhereQueryFunctionEntity.where { length(name) == 1
}.list()*.name.sort()
+ def ilikeLabel = WhereQueryFunctionEntity.where { upper(label) =~
'alp%' }.list()*.name
+ def convertedIntCriteria = new
DetachedCriteria<>(WhereQueryFunctionEntity)
+ convertedIntCriteria.add(new FunctionCallingCriterion('year', new
Query.Equals('created', '2011')))
+ def convertedInt = convertedIntCriteria.list()*.name
+
+ then:
+ trimmed == ['a']
+ oneCharacter == ['a', 'b', 'c', 'd']
+ ilikeLabel == ['a']
+ convertedInt == ['a']
+ }
+
+ void "function comparisons handle null operands, and NotEquals leaves out
null rows as SQL does"() {
+ given:
+ seedEntities()
+
+ when:
+ def nullLabel = WhereQueryFunctionEntity.where { upper(label) == null
}.list()*.name
+ def nonNullLabel = WhereQueryFunctionEntity.where { upper(label) !=
null }.list()*.name.sort()
+ def notAlpha = WhereQueryFunctionEntity.where { upper(label) !=
'ALPHA' }.list()*.name.sort()
+
+ then:
+ nullLabel == ['c']
+ nonNullLabel == ['a', 'b', 'd']
+ notAlpha == ['b', 'd']
+ }
+
+ void "function IN criterion supports projected DetachedCriteria
subquery"() {
+ given:
+ seedEntities()
+ DetachedCriteria<WhereQueryFunctionEntity> sub = new
DetachedCriteria<>(WhereQueryFunctionEntity).build {
+ projections {
+ property('name')
+ }
+ le('amount', 2)
+ }
+ DetachedCriteria<WhereQueryFunctionEntity> query = new
DetachedCriteria<>(WhereQueryFunctionEntity)
+ query.add(new FunctionCallingCriterion('lower', new Query.In('name',
sub)))
+
+ when:
+ def matches = query.list()*.name.sort()
+
+ then:
+ matches == ['a', 'b']
+ }
+
+ void "bulk updateAll and deleteAll support function IN, ILike, and
subquery criteria"() {
Review Comment:
This feature runs two `updateAll` calls; there is no `deleteAll` and no
subquery in it. Several things this PR changes are covered only by specs that
compare query strings or count predicates. All of them pass when executed, so
this is about pinning them:
- `deleteAll` with a function, and a function `in` over a subquery in a bulk
operation.
- Parameter positions after a subquery in a bulk operation: `inList('name',
sub); eq('qty', 1)` followed by `updateAll` fails at 8803d54c23
(`SemanticException` on Hibernate 7, `Could not locate named parameter [p3]` on
Hibernate 5) and works here, for `notIn` too. That fix reaches Hibernate 5,
which has no executed test for it.
- `==` and `!=` against a projected subquery, such as `where { qty ==
max(qty) }`: a `ClassCastException` at 8803d54c23, right here because
`buildSubquery` now hands the subquery's projections to the creator.
`PredicateGeneratorSpec` only checks `predicates.length == 13` for it.
- The association form the guide documents, `year(pets.birthDate) == 2009`,
and `month`, `day`, `hour`, `minute` and `second`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
