Skip to content

Commit 59d4329

Browse files
jeffgbutlerjeffgbutler
committed
Remove useless parentheses in where clauses
This will also help when implementing Having clauses.
1 parent b5189a8 commit 59d4329

File tree

14 files changed

+126
-49
lines changed

14 files changed

+126
-49
lines changed

‎src/main/java/org/mybatis/dynamic/sql/util/FragmentCollector.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ public FragmentCollector merge(FragmentCollector other) {
4242
return this;
4343
}
4444

45+
public String firstFragment() {
46+
return fragments.get(0).fragment();
47+
}
48+
4549
public Stream<String> fragments() {
4650
return fragments.stream()
4751
.map(FragmentAndParameters::fragment);

‎src/main/java/org/mybatis/dynamic/sql/where/render/WhereRenderer.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,22 @@ private Optional<RenderedCriterion> renderWithoutInitialCriterion() {
6868
}
6969

7070
private String calculateWhereClause(FragmentCollector collector) {
71-
return collector.fragments()
72-
.collect(Collectors.joining(" ", "where ", "")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
71+
if (collector.hasMultipleFragments()) {
72+
return collector.fragments()
73+
.collect(Collectors.joining(" ", "where ", "")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
74+
} else {
75+
return "where " + stripEnclosingParenthesesIfPresent(collector.firstFragment()); //$NON-NLS-1$
76+
}
77+
}
78+
79+
private String stripEnclosingParenthesesIfPresent(String fragment) {
80+
// The fragment will have surrounding open/close parentheses if there is more than one rendered condition.
81+
// Since there is only a single fragment, we don't need these in the where clause
82+
if (fragment.startsWith("(") && fragment.endsWith(")")) { //$NON-NLS-1$ //$NON-NLS-2$
83+
return fragment.substring(1, fragment.length() - 1);
84+
} else {
85+
return fragment;
86+
}
7387
}
7488

7589
public static Builder withWhereModel(WhereModel whereModel) {

‎src/test/java/examples/animal/data/AnimalDataTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ void testComplexConditionWithStandaloneWhereAndTableAlias() {
317317
.render(RenderingStrategies.MYBATIS3, ExplicitTableAliasCalculator.of(animalData, "a"));
318318

319319
assertThat(whereClause).hasValueSatisfying(wc -> {
320-
assertThat(wc.getWhereClause()).isEqualTo("where (a.id = #{parameters.p1,jdbcType=INTEGER} or a.body_weight > #{parameters.p2,jdbcType=DOUBLE})");
320+
assertThat(wc.getWhereClause()).isEqualTo("where a.id = #{parameters.p1,jdbcType=INTEGER} or a.body_weight > #{parameters.p2,jdbcType=DOUBLE}");
321321
List<AnimalData> animals = mapper.selectWithWhereClauseAndAlias(wc);
322322
assertThat(animals).hasSize(59);
323323
});

‎src/test/java/examples/joins/ExistsTest.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ class ExistsTest {
5656
void setup() throws Exception {
5757
Class.forName(JDBC_DRIVER);
5858
InputStream is = getClass().getResourceAsStream("/examples/joins/CreateJoinDB.sql");
59+
assert is != null;
5960
try (Connection connection = DriverManager.getConnection(JDBC_URL, "sa", "")) {
6061
ScriptRunner sr = new ScriptRunner(connection);
6162
sr.setLogWriter(null);
@@ -333,8 +334,8 @@ void testWhereExistsOr() {
333334
.render(RenderingStrategies.MYBATIS3);
334335

335336
String expectedStatement = "select im.* from ItemMaster im"
336-
+ " where (exists (select ol.* from OrderLine ol where ol.item_id = im.item_id)"
337-
+ " or im.item_id = #{parameters.p1,jdbcType=INTEGER})"
337+
+ " where exists (select ol.* from OrderLine ol where ol.item_id = im.item_id)"
338+
+ " or im.item_id = #{parameters.p1,jdbcType=INTEGER}"
338339
+ " order by item_id";
339340
assertThat(selectStatement.getSelectStatement()).isEqualTo(expectedStatement);
340341

@@ -372,8 +373,8 @@ void testWhereExistsAnd() {
372373
.render(RenderingStrategies.MYBATIS3);
373374

374375
String expectedStatement = "select im.* from ItemMaster im"
375-
+ " where (exists (select ol.* from OrderLine ol where ol.item_id = im.item_id)"
376-
+ " and im.item_id = #{parameters.p1,jdbcType=INTEGER})"
376+
+ " where exists (select ol.* from OrderLine ol where ol.item_id = im.item_id)"
377+
+ " and im.item_id = #{parameters.p1,jdbcType=INTEGER}"
377378
+ " order by item_id";
378379
assertThat(selectStatement.getSelectStatement()).isEqualTo(expectedStatement);
379380

‎src/test/java/examples/joins/JoinMapperTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ void testMultipleTableJoinWithComplexWhereClause() {
307307

308308
String expectedStatement = "select om.order_id, om.order_date, ol.line_number, im.description, ol.quantity"
309309
+ " from OrderMaster om join OrderLine ol on om.order_id = ol.order_id join ItemMaster im on ol.item_id = im.item_id"
310-
+ " where (om.order_id = #{parameters.p1,jdbcType=INTEGER} and ol.line_number = #{parameters.p2,jdbcType=INTEGER})";
310+
+ " where om.order_id = #{parameters.p1,jdbcType=INTEGER} and ol.line_number = #{parameters.p2,jdbcType=INTEGER}";
311311
assertThat(selectStatement.getSelectStatement()).isEqualTo(expectedStatement);
312312

313313
List<OrderMaster> rows = mapper.selectMany(selectStatement);

‎src/test/java/issues/gh430/NoInitialConditionTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ void testNoInitialConditionWhereMultipleSubs() {
8383
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
8484

8585
String expected = "select column1, column2 from foo where " +
86-
"(column2 = :p1 or column2 = :p2 or column2 = :p3)";
86+
"column2 = :p1 or column2 = :p2 or column2 = :p3";
8787

8888
assertThat(selectStatement.getSelectStatement()).isEqualTo(expected);
8989
}
@@ -101,9 +101,9 @@ void testNoInitialConditionWhereNotMultipleSubs() {
101101
.build()
102102
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
103103

104-
String expected = "select column1, column2 from foo where (not " +
104+
String expected = "select column1, column2 from foo where not " +
105105
"(column2 = :p1 or column2 = :p2 or column2 = :p3) " +
106-
"and column1 < :p4)";
106+
"and column1 < :p4";
107107

108108
assertThat(selectStatement.getSelectStatement()).isEqualTo(expected);
109109
}
@@ -122,8 +122,8 @@ void testNoInitialConditionWhereGroupMultipleSubs() {
122122
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
123123

124124
String expected = "select column1, column2 from foo where " +
125-
"((column2 = :p1 or column2 = :p2 or column2 = :p3) " +
126-
"and column1 < :p4)";
125+
"(column2 = :p1 or column2 = :p2 or column2 = :p3) " +
126+
"and column1 < :p4";
127127

128128
assertThat(selectStatement.getSelectStatement()).isEqualTo(expected);
129129
}
@@ -142,7 +142,7 @@ void testNoInitialConditionWhereCCAndMultipleSubs() {
142142
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
143143

144144
String expected = "select column1, column2 from foo where " +
145-
"(column1 < :p1 and (column2 = :p2 or column2 = :p3 or column2 = :p4))";
145+
"column1 < :p1 and (column2 = :p2 or column2 = :p3 or column2 = :p4)";
146146

147147
assertThat(selectStatement.getSelectStatement()).isEqualTo(expected);
148148
}
@@ -161,7 +161,7 @@ void testNoInitialConditionWhereCCOrMultipleSubs() {
161161
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
162162

163163
String expected = "select column1, column2 from foo where " +
164-
"(column1 < :p1 or (column2 = :p2 or column2 = :p3 or column2 = :p4))";
164+
"column1 < :p1 or (column2 = :p2 or column2 = :p3 or column2 = :p4)";
165165

166166
assertThat(selectStatement.getSelectStatement()).isEqualTo(expected);
167167
}

‎src/test/java/org/mybatis/dynamic/sql/update/UpdateStatementTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void testUpdateParameterWithMultipleCriteria() {
4545
.render(RenderingStrategies.MYBATIS3);
4646

4747
String expected = "update foo set firstName = #{parameters.p1,jdbcType=VARCHAR}, lastName = #{parameters.p2,jdbcType=VARCHAR}, occupation = null "
48-
+ "where (id = #{parameters.p3,jdbcType=INTEGER} or id = #{parameters.p4,jdbcType=INTEGER} or id = #{parameters.p5,jdbcType=INTEGER})";
48+
+ "where id = #{parameters.p3,jdbcType=INTEGER} or id = #{parameters.p4,jdbcType=INTEGER} or id = #{parameters.p5,jdbcType=INTEGER}";
4949

5050
assertAll(
5151
() -> assertThat(updateStatement.getUpdateStatement()).isEqualTo(expected),
@@ -69,7 +69,7 @@ void testUpdateParameterWithMultipleNestedCriteria() {
6969
.render(RenderingStrategies.MYBATIS3);
7070

7171
String expected = "update foo set firstName = #{parameters.p1,jdbcType=VARCHAR}, lastName = #{parameters.p2,jdbcType=VARCHAR}, occupation = null "
72-
+ "where (id = #{parameters.p3,jdbcType=INTEGER} or (id = #{parameters.p4,jdbcType=INTEGER} or id = #{parameters.p5,jdbcType=INTEGER}))";
72+
+ "where id = #{parameters.p3,jdbcType=INTEGER} or (id = #{parameters.p4,jdbcType=INTEGER} or id = #{parameters.p5,jdbcType=INTEGER})";
7373

7474
assertAll(
7575
() -> assertThat(updateStatement.getUpdateStatement()).isEqualTo(expected),

‎src/test/java/org/mybatis/dynamic/sql/where/WhereModelTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ void testThatParameterNameCarriesToSubCriteria() {
3838
.render(RenderingStrategies.MYBATIS3, "myName");
3939

4040
assertThat(whereClause.map(WhereClauseProvider::getWhereClause)).hasValueSatisfying(wc ->
41-
assertThat(wc).isEqualTo("where (id = #{myName.parameters.p1,jdbcType=INTEGER} or id = #{myName.parameters.p2,jdbcType=INTEGER})")
41+
assertThat(wc).isEqualTo("where id = #{myName.parameters.p1,jdbcType=INTEGER} or id = #{myName.parameters.p2,jdbcType=INTEGER}")
4242
);
4343
}
4444
}

‎src/test/java/org/mybatis/dynamic/sql/where/render/OptionalCriterionRenderTest.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ void testOverrideFirstConnector() {
169169

170170
assertThat(whereClause).hasValueSatisfying(wc -> {
171171
assertThat(wc.getParameters()).containsExactly(entry("p1", "fred"), entry("p2", "flintstone"));
172-
assertThat(wc.getWhereClause()).isEqualTo("where (first_name = :p1 or last_name = :p2)");
172+
assertThat(wc.getWhereClause()).isEqualTo("where first_name = :p1 or last_name = :p2");
173173
});
174174
}
175175

@@ -206,8 +206,8 @@ void testWhereExistsOr() {
206206
.build()
207207
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
208208

209-
String expected = "where (exists (select * from person where id = :p1) " +
210-
"or exists (select * from person where id = :p2))";
209+
String expected = "where exists (select * from person where id = :p1) " +
210+
"or exists (select * from person where id = :p2)";
211211

212212
assertThat(whereClause).hasValueSatisfying(wc -> {
213213
assertThat(wc.getParameters()).containsExactly(entry("p1", 3), entry("p2", 4));
@@ -237,9 +237,9 @@ void testWhereExistsOrOr() {
237237
.build()
238238
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
239239

240-
String expected = "where (exists (select * from person where id = :p1) " +
240+
String expected = "where exists (select * from person where id = :p1) " +
241241
"or (exists (select * from person where id = :p2) " +
242-
"or exists (select * from person where id = :p3)))";
242+
"or exists (select * from person where id = :p3))";
243243

244244
assertThat(whereClause).hasValueSatisfying(wc -> {
245245
assertThat(wc.getParameters()).containsExactly(entry("p1", 3), entry("p2", 4), entry("p3", 5));
@@ -263,8 +263,8 @@ void testWhereExistsAnd() {
263263
.build()
264264
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
265265

266-
String expected = "where (exists (select * from person where id = :p1) " +
267-
"and exists (select * from person where id = :p2))";
266+
String expected = "where exists (select * from person where id = :p1) " +
267+
"and exists (select * from person where id = :p2)";
268268

269269
assertThat(whereClause).hasValueSatisfying(wc -> {
270270
assertThat(wc.getParameters()).containsExactly(entry("p1", 3), entry("p2", 4));
@@ -294,9 +294,9 @@ void testWhereExistsAndAnd() {
294294
.build()
295295
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
296296

297-
String expected = "where (exists (select * from person where id = :p1) " +
297+
String expected = "where exists (select * from person where id = :p1) " +
298298
"and (exists (select * from person where id = :p2) " +
299-
"and exists (select * from person where id = :p3)))";
299+
"and exists (select * from person where id = :p3))";
300300

301301
assertThat(whereClause).hasValueSatisfying(wc -> {
302302
assertThat(wc.getParameters()).containsExactly(entry("p1", 3), entry("p2", 4), entry("p3", 5));
@@ -343,7 +343,7 @@ void testCollapsingCriteriaGroup3() {
343343
.build()
344344
.render(RenderingStrategies.SPRING_NAMED_PARAMETER);
345345

346-
String expected = "where (first_name = :p1 or first_name = :p2)";
346+
String expected = "where first_name = :p1 or first_name = :p2";
347347

348348
assertThat(whereClause).hasValueSatisfying(wc -> {
349349
assertThat(wc.getParameters()).containsExactly(entry("p1", "Fred"), entry("p2", "Betty"));

‎src/test/kotlin/examples/kotlin/mybatis3/general/GeneralKotlinTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ class GeneralKotlinTest {
695695
assertThat(updateStatement.updateStatement).isEqualTo(
696696
"update Person" +
697697
" set first_name = #{parameters.p1,jdbcType=VARCHAR}" +
698-
" where (first_name = #{parameters.p2,jdbcType=VARCHAR} or id > #{parameters.p3,jdbcType=INTEGER})"
698+
" where first_name = #{parameters.p2,jdbcType=VARCHAR} or id > #{parameters.p3,jdbcType=INTEGER}"
699699
)
700700

701701
val rows = mapper.update(updateStatement)

0 commit comments

Comments
 (0)