Skip to content

Commit

Permalink
Wrap multiple conditions in brackets in WHERE. (#65)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@repalash
repalash authored Oct 23, 2024
1 parent c5eb1fb commit 5c1eb5f
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 28 deletions.
6 changes: 4 additions & 2 deletions src/builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 420,10 @@ export class QueryBuilder<GenericResultWrapper, IsAsync extends boolean = true>

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

if ((conditions as Array<string>).length > 0) {
return ` WHERE ${(conditions as Array<string>).join(' AND ')}`
if ((conditions as Array<string>).length === 1) return ` WHERE ${(conditions as Array<string>)[0]!.toString()}`

if ((conditions as Array<string>).length > 1) {
return ` WHERE (${(conditions as Array<string>).join(') AND (')})`
}

return ''
Expand Down
22 changes: 13 additions & 9 deletions tests/builder/delete.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 57,7 @@ describe('Delete Builder', () => {
where: ['field = false', 'active = false'],
})

expect(result.query).toEqual('DELETE FROM testTable WHERE field = false AND active = false')
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = false) AND (active = false)')
expect(result.arguments).toEqual(undefined)
expect(result.fetchType).toEqual('ALL')
})
Expand All @@ -71,7 71,7 @@ describe('Delete Builder', () => {
},
})

expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2')
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = ?1) AND (id = ?2)')
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
})
Expand All @@ -86,7 86,7 @@ describe('Delete Builder', () => {
returning: 'id',
})

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

expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id, field')
expect(result.query).toEqual('DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id, field')
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
})
Expand All @@ -114,10 114,12 @@ describe('Delete Builder', () => {
params: ['test', 123],
},
returning: ['id', 'field'],
limit: 10000
limit: 10000,
})

expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id, field LIMIT 10000')
expect(result.query).toEqual(
'DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id, field LIMIT 10000'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
})
Expand All @@ -130,11 132,13 @@ describe('Delete Builder', () => {
params: ['test', 123],
},
returning: ['id', 'field'],
orderBy: "id",
limit: 10000
orderBy: 'id',
limit: 10000,
})

expect(result.query).toEqual('DELETE FROM testTable WHERE field = ?1 AND id = ?2 RETURNING id, field ORDER BY id LIMIT 10000')
expect(result.query).toEqual(
'DELETE FROM testTable WHERE (field = ?1) AND (id = ?2) RETURNING id, field ORDER BY id LIMIT 10000'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
})
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/insert.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 307,7 @@ describe('Insert Builder', () => {

expect(result.query).toEqual(
'INSERT INTO phonebook2 (name, phonenumber, validDate) VALUES (?1, ?2, ?3) ON CONFLICT (name) DO '
'UPDATE SET phonenumber = excluded.phonenumber, validDate = excluded.validDate WHERE excluded.validDate > Date(now()) AND active = true'
'UPDATE SET phonenumber = excluded.phonenumber, validDate = excluded.validDate WHERE (excluded.validDate > Date(now())) AND (active = true)'
)
expect(result.arguments).toEqual(['Alice', '704-555-1212', '2018-05-08'])
expect(result.fetchType).toEqual('ONE')
Expand Down
47 changes: 37 additions & 10 deletions tests/builder/select.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 184,7 @@ describe('Select Builder', () => {
where: ['field = true', 'active = false'],
}),
]) {
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = true AND active = false LIMIT 1')
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = true) AND (active = false) LIMIT 1')
expect(result.arguments).toEqual(undefined)
expect(result.fetchType).toEqual('ONE')
}
Expand Down Expand Up @@ -526,7 526,7 @@ describe('Select Builder', () => {
.where('test = ?', 123)
.getQueryAll(),
]) {
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ? AND test = ?')
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = ?) AND (test = ?)')
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
}
Expand All @@ -551,7 551,7 @@ describe('Select Builder', () => {
.groupBy('type')
.getQueryAll(),
]) {
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type')
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type')
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
}
Expand All @@ -577,7 577,7 @@ describe('Select Builder', () => {
.groupBy('day')
.getQueryAll(),
]) {
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type, day')
expect(result.query).toEqual('SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type, day')
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
}
Expand Down Expand Up @@ -605,7 605,7 @@ describe('Select Builder', () => {
.getQueryAll(),
]) {
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type HAVING COUNT(trackid) > 15'
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type HAVING COUNT(trackid) > 15'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
Expand Down Expand Up @@ -635,7 635,7 @@ describe('Select Builder', () => {
.getQueryAll(),
]) {
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type HAVING COUNT(trackid) > 15 AND COUNT(trackid) < 30'
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type HAVING COUNT(trackid) > 15 AND COUNT(trackid) < 30'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
Expand Down Expand Up @@ -663,7 663,9 @@ describe('Select Builder', () => {
.orderBy('id')
.getQueryAll(),
]) {
expect(result.query).toEqual('SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type ORDER BY id')
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type ORDER BY id'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
}
Expand Down Expand Up @@ -692,7 694,7 @@ describe('Select Builder', () => {
.getQueryAll(),
]) {
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type ORDER BY id, timestamp'
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type ORDER BY id, timestamp'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
Expand Down Expand Up @@ -725,7 727,7 @@ describe('Select Builder', () => {
.getQueryAll(),
]) {
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type ORDER BY id ASC, timestamp DESC'
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type ORDER BY id ASC, timestamp DESC'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
Expand Down Expand Up @@ -756,10 758,35 @@ describe('Select Builder', () => {
.getQueryAll(),
]) {
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE field = ?1 AND test = ?2 GROUP BY type LIMIT 10 OFFSET 15'
'SELECT * FROM testTable WHERE (field = ?1) AND (test = ?2) GROUP BY type LIMIT 10 OFFSET 15'
)
expect(result.arguments).toEqual(['test', 123])
expect(result.fetchType).toEqual('ALL')
}
})

test('select with multiple where with OR conditions', async () => {
for (const result of [
new QuerybuilderTest().fetchAll({
tableName: 'testTable',
fields: '*',
where: {
conditions: ['field = ?1 OR test = ?2', 'test = ?3 OR field = ?4'],
params: ['test', 123, 456, 'test'],
},
}),
new QuerybuilderTest()
.select('testTable')
.fields('*')
.where('field = ?1 OR test = ?2', ['test', 123])
.where('test = ?3 OR field = ?4', [456, 'test'])
.getQueryAll(),
]) {
expect(result.query).toEqual(
'SELECT * FROM testTable WHERE (field = ?1 OR test = ?2) AND (test = ?3 OR field = ?4)'
)
expect(result.arguments).toEqual(['test', 123, 456, 'test'])
expect(result.fetchType).toEqual('ALL')
}
})
})
12 changes: 6 additions & 6 deletions tests/builder/update.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 43,7 @@ describe('Update Builder', () => {
where: ['field = true', 'active = true'],
})

expect(result.query).toEqual('UPDATE testTable SET my_field = ?1 WHERE field = true AND active = true')
expect(result.query).toEqual('UPDATE testTable SET my_field = ?1 WHERE (field = true) AND (active = true)')
expect(result.arguments).toEqual(['test_data'])
expect(result.fetchType).toEqual('ALL')
})
Expand Down Expand Up @@ -117,7 117,7 @@ describe('Update Builder', () => {
},
})

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

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

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

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

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

0 comments on commit 5c1eb5f

Please sign in to comment.