Skip to content

Commit 5c1eb5f

Browse files
repalashrepalash
authored
Wrap multiple conditions in brackets in WHERE. (#65)
A sample fix for #64 Adds brackets around the conditions in where when using multiple conditions. Without any kind of tokenisation, this requires adding brackets even where not required. I have updated the other tests for this.
1 parent c5eb1fb commit 5c1eb5f

File tree

5 files changed

+61
-28
lines changed

5 files changed

+61
-28
lines changed

‎src/builder.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,10 @@ export class QueryBuilder<GenericResultWrapper, IsAsync extends boolean = true>
420420

421421
if (typeof conditions === 'string') return ` WHERE ${conditions.toString()}`
422422

423-
if ((conditions as Array<string>).length > 0) {
424-
return ` WHERE ${(conditions as Array<string>).join(' AND ')}`
423+
if ((conditions as Array<string>).length === 1) return ` WHERE ${(conditions as Array<string>)[0]!.toString()}`
424+
425+
if ((conditions as Array<string>).length > 1) {
426+
return ` WHERE (${(conditions as Array<string>).join(') AND (')})`
425427
}
426428

427429
return ''

‎tests/builder/delete.test.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('Delete Builder', () => {
5757
where: ['field = false', 'active = false'],
5858
})
5959

60-
expect(result.query).toEqual('DELETE FROM testTable WHERE field = false AND active = false')
60+
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = false) AND (active = false)')
6161
expect(result.arguments).toEqual(undefined)
6262
expect(result.fetchType).toEqual('ALL')
6363
})
@@ -71,7 +71,7 @@ describe('Delete Builder', () => {
7171
},
7272
})
7373

74-
expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2')
74+
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = ?1) AND (id = ?2)')
7575
expect(result.arguments).toEqual(['test', 123])
7676
expect(result.fetchType).toEqual('ALL')
7777
})
@@ -86,7 +86,7 @@ describe('Delete Builder', () => {
8686
returning: 'id',
8787
})
8888

89-
expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id')
89+
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id')
9090
expect(result.arguments).toEqual(['test', 123])
9191
expect(result.fetchType).toEqual('ALL')
9292
})
@@ -101,7 +101,7 @@ describe('Delete Builder', () => {
101101
returning: ['id', 'field'],
102102
})
103103

104-
expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id, field')
104+
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id, field')
105105
expect(result.arguments).toEqual(['test', 123])
106106
expect(result.fetchType).toEqual('ALL')
107107
})
@@ -114,10 +114,12 @@ describe('Delete Builder', () => {
114114
params: ['test', 123],
115115
},
116116
returning: ['id', 'field'],
117-
limit: 10000
117+
limit: 10000,
118118
})
119119

120-
expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id, field LIMIT 10000')
120+
expect(result.query).toEqual(
121+
'DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id, field LIMIT 10000'
122+
)
121123
expect(result.arguments).toEqual(['test', 123])
122124
expect(result.fetchType).toEqual('ALL')
123125
})
@@ -130,11 +132,13 @@ describe('Delete Builder', () => {
130132
params: ['test', 123],
131133
},
132134
returning: ['id', 'field'],
133-
orderBy: "id",
134-
limit: 10000
135+
orderBy: 'id',
136+
limit: 10000,
135137
})
136138

137-
expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id, field ORDER BY id LIMIT 10000')
139+
expect(result.query).toEqual(
140+
'DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id, field ORDER BY id LIMIT 10000'
141+
)
138142
expect(result.arguments).toEqual(['test', 123])
139143
expect(result.fetchType).toEqual('ALL')
140144
})

‎tests/builder/insert.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ describe('Insert Builder', () => {
307307

308308
expect(result.query).toEqual(
309309
'INSERT INTO phonebook2 (name, phonenumber, validDate) VALUES (?1, ?2, ?3) ON CONFLICT (name) DO ' +
310-
'UPDATE SET phonenumber = excluded.phonenumber, validDate = excluded.validDate WHERE excluded.validDate > Date(now()) AND active = true'
310+
'UPDATE SET phonenumber = excluded.phonenumber, validDate = excluded.validDate WHERE (excluded.validDate > Date(now())) AND (active = true)'
311311
)
312312
expect(result.arguments).toEqual(['Alice', '704-555-1212', '2018-05-08'])
313313
expect(result.fetchType).toEqual('ONE')

‎tests/builder/select.test.ts‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ describe('Select Builder', () => {
184184
where: ['field = true', 'active = false'],
185185
}),
186186
]) {
187-
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = true AND active = false LIMIT 1')
187+
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = true) AND (active = false) LIMIT 1')
188188
expect(result.arguments).toEqual(undefined)
189189
expect(result.fetchType).toEqual('ONE')
190190
}
@@ -526,7 +526,7 @@ describe('Select Builder', () => {
526526
.where('test = ?', 123)
527527
.getQueryAll(),
528528
]) {
529-
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ? AND test = ?')
529+
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = ?) AND (test = ?)')
530530
expect(result.arguments).toEqual(['test', 123])
531531
expect(result.fetchType).toEqual('ALL')
532532
}
@@ -551,7 +551,7 @@ describe('Select Builder', () => {
551551
.groupBy('type')
552552
.getQueryAll(),
553553
]) {
554-
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type')
554+
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type')
555555
expect(result.arguments).toEqual(['test', 123])
556556
expect(result.fetchType).toEqual('ALL')
557557
}
@@ -577,7 +577,7 @@ describe('Select Builder', () => {
577577
.groupBy('day')
578578
.getQueryAll(),
579579
]) {
580-
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type, day')
580+
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type, day')
581581
expect(result.arguments).toEqual(['test', 123])
582582
expect(result.fetchType).toEqual('ALL')
583583
}
@@ -605,7 +605,7 @@ describe('Select Builder', () => {
605605
.getQueryAll(),
606606
]) {
607607
expect(result.query).toEqual(
608-
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type HAVING COUNT(trackid) > 15'
608+
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type HAVING COUNT(trackid) > 15'
609609
)
610610
expect(result.arguments).toEqual(['test', 123])
611611
expect(result.fetchType).toEqual('ALL')
@@ -635,7 +635,7 @@ describe('Select Builder', () => {
635635
.getQueryAll(),
636636
]) {
637637
expect(result.query).toEqual(
638-
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type HAVING COUNT(trackid) > 15 AND COUNT(trackid) < 30'
638+
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type HAVING COUNT(trackid) > 15 AND COUNT(trackid) < 30'
639639
)
640640
expect(result.arguments).toEqual(['test', 123])
641641
expect(result.fetchType).toEqual('ALL')
@@ -663,7 +663,9 @@ describe('Select Builder', () => {
663663
.orderBy('id')
664664
.getQueryAll(),
665665
]) {
666-
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type ORDER BY id')
666+
expect(result.query).toEqual(
667+
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type ORDER BY id'
668+
)
667669
expect(result.arguments).toEqual(['test', 123])
668670
expect(result.fetchType).toEqual('ALL')
669671
}
@@ -692,7 +694,7 @@ describe('Select Builder', () => {
692694
.getQueryAll(),
693695
]) {
694696
expect(result.query).toEqual(
695-
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type ORDER BY id, timestamp'
697+
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type ORDER BY id, timestamp'
696698
)
697699
expect(result.arguments).toEqual(['test', 123])
698700
expect(result.fetchType).toEqual('ALL')
@@ -725,7 +727,7 @@ describe('Select Builder', () => {
725727
.getQueryAll(),
726728
]) {
727729
expect(result.query).toEqual(
728-
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type ORDER BY id ASC, timestamp DESC'
730+
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type ORDER BY id ASC, timestamp DESC'
729731
)
730732
expect(result.arguments).toEqual(['test', 123])
731733
expect(result.fetchType).toEqual('ALL')
@@ -756,10 +758,35 @@ describe('Select Builder', () => {
756758
.getQueryAll(),
757759
]) {
758760
expect(result.query).toEqual(
759-
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type LIMIT 10 OFFSET 15'
761+
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type LIMIT 10 OFFSET 15'
760762
)
761763
expect(result.arguments).toEqual(['test', 123])
762764
expect(result.fetchType).toEqual('ALL')
763765
}
764766
})
767+
768+
test('select with multiple where with OR conditions', async () => {
769+
for (const result of [
770+
new QuerybuilderTest().fetchAll({
771+
tableName: 'testTable',
772+
fields: '*',
773+
where: {
774+
conditions: ['field = ?1 OR test = ?2', 'test = ?3 OR field = ?4'],
775+
params: ['test', 123, 456, 'test'],
776+
},
777+
}),
778+
new QuerybuilderTest()
779+
.select('testTable')
780+
.fields('*')
781+
.where('field = ?1 OR test = ?2', ['test', 123])
782+
.where('test = ?3 OR field = ?4', [456, 'test'])
783+
.getQueryAll(),
784+
]) {
785+
expect(result.query).toEqual(
786+
'SELECT * FROM testTable WHERE (field = ?1 OR test = ?2) AND (test = ?3 OR field = ?4)'
787+
)
788+
expect(result.arguments).toEqual(['test', 123, 456, 'test'])
789+
expect(result.fetchType).toEqual('ALL')
790+
}
791+
})
765792
})

‎tests/builder/update.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('Update Builder', () => {
4343
where: ['field = true', 'active = true'],
4444
})
4545

46-
expect(result.query).toEqual('UPDATE testTable SET my_field = ?1 WHERE field = true AND active = true')
46+
expect(result.query).toEqual('UPDATE testTable SET my_field = ?1 WHERE (field = true) AND (active = true)')
4747
expect(result.arguments).toEqual(['test_data'])
4848
expect(result.fetchType).toEqual('ALL')
4949
})
@@ -117,7 +117,7 @@ describe('Update Builder', () => {
117117
},
118118
})
119119

120-
expect(result.query).toEqual('UPDATE testTable SET my_field = ?3, another = ?4 WHERE field = ?1 AND id = ?2')
120+
expect(result.query).toEqual('UPDATE testTable SET my_field = ?3, another = ?4 WHERE (field = ?1) AND (id = ?2)')
121121
expect(result.arguments).toEqual(['test', 345, 'test_update', 123])
122122
expect(result.fetchType).toEqual('ALL')
123123
})
@@ -137,7 +137,7 @@ describe('Update Builder', () => {
137137
})
138138

139139
expect(result.query).toEqual(
140-
'UPDATE testTable SET my_field = ?3, another = ?4 WHERE field = ?1 AND id = ?2 RETURNING id'
140+
'UPDATE testTable SET my_field = ?3, another = ?4 WHERE (field = ?1) AND (id = ?2) RETURNING id'
141141
)
142142
expect(result.arguments).toEqual(['test', 345, 'test_update', 123])
143143
expect(result.fetchType).toEqual('ALL')
@@ -158,7 +158,7 @@ describe('Update Builder', () => {
158158
})
159159

160160
expect(result.query).toEqual(
161-
'UPDATE testTable SET my_field = ?3, another = ?4 WHERE field = ?1 AND id = ?2 RETURNING id, field'
161+
'UPDATE testTable SET my_field = ?3, another = ?4 WHERE (field = ?1) AND (id = ?2) RETURNING id, field'
162162
)
163163
expect(result.arguments).toEqual(['test', 345, 'test_update', 123])
164164
expect(result.fetchType).toEqual('ALL')
@@ -180,7 +180,7 @@ describe('Update Builder', () => {
180180
})
181181

182182
expect(result.query).toEqual(
183-
'UPDATE OR IGNORE testTable SET my_field = ?3, another = ?4 WHERE field = ?1 AND id = ?2 RETURNING id, field'
183+
'UPDATE OR IGNORE testTable SET my_field = ?3, another = ?4 WHERE (field = ?1) AND (id = ?2) RETURNING id, field'
184184
)
185185
expect(result.arguments).toEqual(['test', 345, 'test_update', 123])
186186
expect(result.fetchType).toEqual('ALL')
@@ -202,7 +202,7 @@ describe('Update Builder', () => {
202202
})
203203

204204
expect(result.query).toEqual(
205-
'UPDATE OR REPLACE testTable SET my_field = ?3, another = ?4 WHERE field = ?1 AND id = ?2 RETURNING id, field'
205+
'UPDATE OR REPLACE testTable SET my_field = ?3, another = ?4 WHERE (field = ?1) AND (id = ?2) RETURNING id, field'
206206
)
207207
expect(result.arguments).toEqual(['test', 345, 'test_update', 123])
208208
expect(result.fetchType).toEqual('ALL')

0 commit comments

Comments
 (0)