Skip to content

Commit deef759

Browse files
erizocosmicoerizocosmico
authored
Merge pull request src-d#684 from kuba--/fix-or-filter
Fix OR filtering on different columns.
2 parents 115a755 + 6c40d3c commit deef759

File tree

4 files changed

+89
-168
lines changed

4 files changed

+89
-168
lines changed

‎commit_files_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,25 @@ func TestCommitFilesIndex(t *testing.T) {
7676
)
7777
}
7878

79+
func TestCommitFilesOr(t *testing.T) {
80+
testTableIndex(
81+
t,
82+
new(commitFilesTable),
83+
[]sql.Expression{
84+
expression.NewOr(
85+
expression.NewEquals(
86+
expression.NewGetField(1, sql.Text, "commit_hash", false),
87+
expression.NewLiteral("1669dce138d9b841a518c64b10914d88f5e488ea", sql.Text),
88+
),
89+
expression.NewEquals(
90+
expression.NewGetField(2, sql.Text, "file_path", false),
91+
expression.NewLiteral("go/example.go", sql.Text),
92+
),
93+
),
94+
},
95+
)
96+
}
97+
7998
func TestEncodeCommitFileIndexKey(t *testing.T) {
8099
require := require.New(t)
81100

‎filters.go‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -224,74 +224,12 @@ func classifyFilters(
224224
continue
225225
}
226226
}
227-
case *expression.Or:
228-
exprs := unfoldOrs(f)
229-
// check all unfolded exprs can be handled, if not we have to
230-
// resort to treating them as conditions
231-
valid := true
232-
Loop:
233-
for _, e := range exprs {
234-
switch e := e.(type) {
235-
case *expression.Equals:
236-
if !canHandleEquals(schema, table, e) {
237-
valid = false
238-
break Loop
239-
}
240-
case *expression.In:
241-
if !canHandleIn(schema, table, e) {
242-
valid = false
243-
break Loop
244-
}
245-
default:
246-
valid = false
247-
break Loop
248-
}
249-
}
250-
251-
if !valid {
252-
conditions = append(conditions, f)
253-
continue
254-
}
255-
256-
// by definition there can be no conditions
257-
sels, _, err := classifyFilters(schema, table, exprs, handledCols...)
258-
if err != nil {
259-
return nil, nil, err
260-
}
261-
262-
for k, v := range sels {
263-
var values selector
264-
for _, vals := range v {
265-
values = append(values, vals...)
266-
}
267-
selectors[k] = append(selectors[k], values)
268-
}
269-
270-
continue
271227
}
272228
conditions = append(conditions, f)
273229
}
274230
return selectors, conditions, nil
275231
}
276232

277-
func unfoldOrs(or *expression.Or) []sql.Expression {
278-
var exprs []sql.Expression
279-
280-
if left, ok := or.Left.(*expression.Or); ok {
281-
exprs = append(exprs, unfoldOrs(left)...)
282-
} else {
283-
exprs = append(exprs, or.Left)
284-
}
285-
286-
if right, ok := or.Right.(*expression.Or); ok {
287-
exprs = append(exprs, unfoldOrs(right)...)
288-
} else {
289-
exprs = append(exprs, or.Right)
290-
}
291-
292-
return exprs
293-
}
294-
295233
type iteratorBuilder func(selectors) (sql.RowIter, error)
296234

297235
// rowIterWithSelectors implements all the boilerplate of WithProjectAndFilters

‎filters_test.go‎

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -308,106 +308,6 @@ func TestClassifyFilters(t *testing.T) {
308308
),
309309
false,
310310
},
311-
{
312-
expression.NewOr(
313-
expression.NewOr(
314-
expression.NewEquals(
315-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
316-
expression.NewLiteral(1, sql.Int64),
317-
),
318-
expression.NewEquals(
319-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
320-
expression.NewLiteral(2, sql.Int64),
321-
),
322-
),
323-
expression.NewOr(
324-
expression.NewEquals(
325-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
326-
expression.NewLiteral(3, sql.Int64),
327-
),
328-
expression.NewEquals(
329-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
330-
expression.NewLiteral(4, sql.Int64),
331-
),
332-
),
333-
),
334-
true,
335-
},
336-
{
337-
expression.NewOr(
338-
expression.NewIn(
339-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
340-
expression.NewTuple(
341-
expression.NewLiteral(10, sql.Int64),
342-
expression.NewLiteral(11, sql.Int64),
343-
),
344-
),
345-
expression.NewOr(
346-
expression.NewIn(
347-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
348-
expression.NewTuple(
349-
expression.NewLiteral(12, sql.Int64),
350-
expression.NewLiteral(13, sql.Int64),
351-
),
352-
),
353-
expression.NewEquals(
354-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
355-
expression.NewLiteral(14, sql.Int64),
356-
),
357-
),
358-
),
359-
true,
360-
},
361-
{
362-
expression.NewOr(
363-
expression.NewOr(
364-
expression.NewEquals(
365-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
366-
expression.NewLiteral(1, sql.Int64),
367-
),
368-
expression.NewGreaterThan(
369-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
370-
expression.NewLiteral(2, sql.Int64),
371-
),
372-
),
373-
expression.NewOr(
374-
expression.NewEquals(
375-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
376-
expression.NewLiteral(3, sql.Int64),
377-
),
378-
expression.NewEquals(
379-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
380-
expression.NewLiteral(4, sql.Int64),
381-
),
382-
),
383-
),
384-
false,
385-
},
386-
{
387-
expression.NewOr(
388-
expression.NewIn(
389-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
390-
expression.NewTuple(
391-
expression.NewLiteral(10, sql.Int64),
392-
expression.NewLiteral(11, sql.Int64),
393-
),
394-
),
395-
expression.NewOr(
396-
expression.NewIn(
397-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
398-
expression.NewTuple(
399-
expression.NewLiteral(12, sql.Int64),
400-
expression.NewLiteral(13, sql.Int64),
401-
),
402-
),
403-
expression.NewGreaterThan(
404-
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
405-
expression.NewLiteral(2, sql.Int64),
406-
),
407-
),
408-
),
409-
false,
410-
},
411311
}
412312
schema := sql.Schema{
413313
{Name: "a", Source: "foo"},
@@ -431,8 +331,6 @@ func TestClassifyFilters(t *testing.T) {
431331
"a": []selector{
432332
selector{1},
433333
selector{5, 6, 7},
434-
selector{1, 2, 3, 4},
435-
selector{10, 11, 12, 13, 14},
436334
},
437335
}, sels)
438336

‎integration_test.go‎

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,72 @@ func TestIntegration(t *testing.T) {
165165
{"6ecf0ef2c2dffb796033e5a02219af86ec6584e5", int32(9)},
166166
},
167167
},
168+
{
169+
`SELECT commit_hash, file_path
170+
FROM commit_files
171+
WHERE commit_hash='1669dce138d9b841a518c64b10914d88f5e488ea' OR file_path = 'go/example.go'`,
172+
[]sql.Row{
173+
{"e8d3ffab552895c19b9fcf7aa264d277cde33881", "go/example.go"},
174+
{"6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "go/example.go"},
175+
{"918c48b83bd081e863dbe1b80f8998f058cd8294", "go/example.go"},
176+
{"1669dce138d9b841a518c64b10914d88f5e488ea", ".gitignore"},
177+
{"1669dce138d9b841a518c64b10914d88f5e488ea", "CHANGELOG"},
178+
{"1669dce138d9b841a518c64b10914d88f5e488ea", "LICENSE"},
179+
{"1669dce138d9b841a518c64b10914d88f5e488ea", "binary.jpg"},
180+
},
181+
},
182+
{
183+
`SELECT commit_hash, file_path
184+
FROM commit_files
185+
WHERE commit_hash='1669dce138d9b841a518c64b10914d88f5e488ea' AND file_path = 'binary.jpg'`,
186+
[]sql.Row{
187+
{"1669dce138d9b841a518c64b10914d88f5e488ea", "binary.jpg"},
188+
},
189+
},
190+
{
191+
`SELECT commit_hash, file_path
192+
FROM commit_files
193+
WHERE NOT commit_hash = '1669dce138d9b841a518c64b10914d88f5e488ea' AND file_path = 'go/example.go'`,
194+
[]sql.Row{
195+
{"e8d3ffab552895c19b9fcf7aa264d277cde33881", "go/example.go"},
196+
{"6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "go/example.go"},
197+
{"918c48b83bd081e863dbe1b80f8998f058cd8294", "go/example.go"},
198+
},
199+
},
200+
{
201+
`SELECT blob_hash, tree_hash
202+
FROM commit_files
203+
WHERE tree_hash='eba74343e2f15d62adedfd8c883ee0262b5c8021' OR blob_hash = 'd3ff53e0564a9f87d8e84b6e28e5060e517008aa'`,
204+
[]sql.Row{
205+
{"32858aad3c383ed1ff0a0f9bdf231d54a00c9e88", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
206+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
207+
{"c192bd6a24ea1ab01d78686e417c8bdc7c3d197f", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
208+
{"d5c0f4ab811897cadf03aec358ae60d21f91c50d", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
209+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "a8d315b2b1c615d43042c3a62402b8a54288cf5c"},
210+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "fb72698cab7617ac416264415f13224dfd7a165e"},
211+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "c2d30fa8ef288618f65f6eed6e168e0d514886f4"},
212+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "4d081c50e250fa32ea8b1313cf8bb7c2ad7627fd"},
213+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "c2d30fa8ef288618f65f6eed6e168e0d514886f4"},
214+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "dbd3641b371024f44d0e469a9c8f5457b0660de1"},
215+
},
216+
},
217+
{
218+
`SELECT blob_hash, tree_hash
219+
FROM commit_files
220+
WHERE tree_hash='eba74343e2f15d62adedfd8c883ee0262b5c8021' AND blob_hash = 'd3ff53e0564a9f87d8e84b6e28e5060e517008aa'`,
221+
[]sql.Row{
222+
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
223+
},
224+
},
225+
{
226+
`SELECT commit_hash, tree_hash
227+
FROM commits
228+
WHERE commit_author_email='daniel@lordran.local' OR commit_hash='b029517f6300c2da0f4b651b8642506cd6aaf45d'`,
229+
[]sql.Row{
230+
{"b029517f6300c2da0f4b651b8642506cd6aaf45d", "aa9b383c260e1d05fbbf6b30a02914555e20c725"},
231+
{"b8e471f58bcbca63b07bda20e428190409c2db47", "c2d30fa8ef288618f65f6eed6e168e0d514886f4"},
232+
},
233+
},
168234
{
169235
`SELECT MONTH(committer_when) as month,
170236
r.repository_id as repo_id,
@@ -248,10 +314,10 @@ func TestIntegration(t *testing.T) {
248314
SELECT language, COUNT(repository_id) AS repository_count
249315
FROM (
250316
SELECT DISTINCT r.repository_id, LANGUAGE(t.tree_entry_name, b.blob_content) AS language
251-
FROM refs r
252-
JOIN commits c ON r.commit_hash = c.commit_hash
253-
NATURAL JOIN commit_trees
254-
NATURAL JOIN tree_entries t
317+
FROM refs r
318+
JOIN commits c ON r.commit_hash = c.commit_hash
319+
NATURAL JOIN commit_trees
320+
NATURAL JOIN tree_entries t
255321
NATURAL JOIN blobs b
256322
WHERE language IS NOT NULL
257323
) AS q1

0 commit comments

Comments
 (0)