Skip to content

Commit 799f2f9

Browse files
committed
Fix exists() cross-join caused by duplicate CriteriaQuery root
AbstractHibernateGormStaticApi.exists() called criteriaQuery.from() twice, creating a second query root that produced a cartesian product. The generated SQL selected count(alias0) from Table alias1, Table alias0 where alias1.id=?, scanning the entire table for every matching row instead of a simple count. Reuse the existing queryRoot variable for the count select expression. Fixes #14334 Assisted-by: Claude Code <Claude@Claude.ai>
1 parent 63beaf3 commit 799f2f9

File tree

2 files changed

+123
-1
lines changed

2 files changed

+123
-1
lines changed

grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/AbstractHibernateGormStaticApi.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ abstract class AbstractHibernateGormStaticApi<D> extends GormStaticApi<D> {
245245
//TODO: Remove explicit type cast once GROOVY-9460
246246
criteriaBuilder.equal((Expression<?>) idProp, id)
247247
)
248-
criteriaQuery.select(criteriaBuilder.count(criteriaQuery.from(persistentEntity.javaClass)))
248+
criteriaQuery.select(criteriaBuilder.count(queryRoot))
249249
Query criteria = session.createQuery(criteriaQuery)
250250
HibernateHqlQuery hibernateHqlQuery = new HibernateHqlQuery(
251251
hibernateSession, persistentEntity, criteria)
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.grails.orm.hibernate
20+
21+
import grails.gorm.annotation.Entity
22+
import grails.gorm.transactions.Rollback
23+
import org.grails.datastore.mapping.core.DatastoreUtils
24+
import org.grails.orm.hibernate.cfg.Settings
25+
import org.hibernate.resource.jdbc.spi.StatementInspector
26+
import org.springframework.transaction.PlatformTransactionManager
27+
import spock.lang.AutoCleanup
28+
import spock.lang.Issue
29+
import spock.lang.Shared
30+
import spock.lang.Specification
31+
32+
@Issue('https://github.com/apache/grails-core/issues/14334')
33+
class ExistsCrossJoinSpec extends Specification {
34+
35+
@Shared SqlCapture sqlCapture = new SqlCapture()
36+
37+
@Shared @AutoCleanup HibernateDatastore hibernateDatastore = new HibernateDatastore(
38+
DatastoreUtils.createPropertyResolver(
39+
(Settings.SETTING_DB_CREATE): 'create-drop',
40+
'hibernate.session_factory.statement_inspector': sqlCapture
41+
),
42+
ExistsItem
43+
)
44+
@Shared PlatformTransactionManager transactionManager = hibernateDatastore.getTransactionManager()
45+
46+
@Rollback
47+
void "exists returns true for existing entity"() {
48+
given:
49+
ExistsItem item = new ExistsItem(name: 'alpha').save(flush: true)
50+
51+
expect:
52+
ExistsItem.exists(item.id)
53+
}
54+
55+
@Rollback
56+
void "exists returns false for non-existent id"() {
57+
expect:
58+
!ExistsItem.exists(99999)
59+
}
60+
61+
@Rollback
62+
void "exists does not produce a cross-join"() {
63+
given:
64+
new ExistsItem(name: 'one').save(flush: true)
65+
new ExistsItem(name: 'two').save(flush: true)
66+
new ExistsItem(name: 'three').save(flush: true)
67+
68+
when:
69+
sqlCapture.clear()
70+
ExistsItem item = new ExistsItem(name: 'target').save(flush: true)
71+
sqlCapture.clear()
72+
ExistsItem.exists(item.id)
73+
74+
then: "the SQL should contain only a single FROM clause (no cross-join)"
75+
sqlCapture.statements.any { it.toLowerCase().contains('select count') }
76+
77+
and: "there should be exactly one table reference in the FROM clause"
78+
String countSql = sqlCapture.statements.find { it.toLowerCase().contains('select count') }
79+
countSql != null
80+
// A cross-join would have the table name appearing twice after 'from'
81+
// e.g. "from exists_item x0_, exists_item x1_" vs correct "from exists_item x0_"
82+
countSql.toLowerCase().split('cross join').length == 1
83+
// Verify no comma-join pattern (two table aliases after FROM)
84+
!countSql.toLowerCase().matches(/.*from\s+\S+\s+\S+\s*,\s*\S+\s+\S+.*/)
85+
}
86+
87+
@Rollback
88+
void "exists with multiple rows returns correct result"() {
89+
given: "multiple entities in the table"
90+
ExistsItem target = new ExistsItem(name: 'target').save(flush: true)
91+
new ExistsItem(name: 'other1').save(flush: true)
92+
new ExistsItem(name: 'other2').save(flush: true)
93+
new ExistsItem(name: 'other3').save(flush: true)
94+
new ExistsItem(name: 'other4').save(flush: true)
95+
96+
expect: "exists returns correct results"
97+
ExistsItem.exists(target.id)
98+
!ExistsItem.exists(99999)
99+
}
100+
101+
/**
102+
* Captures SQL statements executed by Hibernate for inspection in tests.
103+
*/
104+
static class SqlCapture implements StatementInspector {
105+
final List<String> statements = Collections.synchronizedList(new ArrayList<String>())
106+
107+
@Override
108+
String inspect(String sql) {
109+
statements.add(sql)
110+
return sql
111+
}
112+
113+
void clear() {
114+
statements.clear()
115+
}
116+
}
117+
}
118+
119+
@Entity
120+
class ExistsItem {
121+
String name
122+
}

0 commit comments

Comments
 (0)