Skip to content

Commit

Permalink
Fix OR filtering on different columns.
Browse files Browse the repository at this point in the history 
Signed-off-by: kuba-- <[email protected]>
  • Loading branch information
kuba-- committed Jan 24, 2019
1 parent 115a755 commit 6c40d3c
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 168 deletions.
19 changes: 19 additions & 0 deletions commit_files_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,25 @@ func TestCommitFilesIndex(t *testing.T) {
)
}

func TestCommitFilesOr(t *testing.T) {
testTableIndex(
t,
new(commitFilesTable),
[]sql.Expression{
expression.NewOr(
expression.NewEquals(
expression.NewGetField(1, sql.Text, "commit_hash", false),
expression.NewLiteral("1669dce138d9b841a518c64b10914d88f5e488ea", sql.Text),
),
expression.NewEquals(
expression.NewGetField(2, sql.Text, "file_path", false),
expression.NewLiteral("go/example.go", sql.Text),
),
),
},
)
}

func TestEncodeCommitFileIndexKey(t *testing.T) {
require := require.New(t)

Expand Down
62 changes: 0 additions & 62 deletions filters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,74 +224,12 @@ func classifyFilters(
continue
}
}
case *expression.Or:
exprs := unfoldOrs(f)
// check all unfolded exprs can be handled, if not we have to
// resort to treating them as conditions
valid := true
Loop:
for _, e := range exprs {
switch e := e.(type) {
case *expression.Equals:
if !canHandleEquals(schema, table, e) {
valid = false
break Loop
}
case *expression.In:
if !canHandleIn(schema, table, e) {
valid = false
break Loop
}
default:
valid = false
break Loop
}
}

if !valid {
conditions = append(conditions, f)
continue
}

// by definition there can be no conditions
sels, _, err := classifyFilters(schema, table, exprs, handledCols...)
if err != nil {
return nil, nil, err
}

for k, v := range sels {
var values selector
for _, vals := range v {
values = append(values, vals...)
}
selectors[k] = append(selectors[k], values)
}

continue
}
conditions = append(conditions, f)
}
return selectors, conditions, nil
}

func unfoldOrs(or *expression.Or) []sql.Expression {
var exprs []sql.Expression

if left, ok := or.Left.(*expression.Or); ok {
exprs = append(exprs, unfoldOrs(left)...)
} else {
exprs = append(exprs, or.Left)
}

if right, ok := or.Right.(*expression.Or); ok {
exprs = append(exprs, unfoldOrs(right)...)
} else {
exprs = append(exprs, or.Right)
}

return exprs
}

type iteratorBuilder func(selectors) (sql.RowIter, error)

// rowIterWithSelectors implements all the boilerplate of WithProjectAndFilters
Expand Down
102 changes: 0 additions & 102 deletions filters_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,106 +308,6 @@ func TestClassifyFilters(t *testing.T) {
),
false,
},
{
expression.NewOr(
expression.NewOr(
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(1, sql.Int64),
),
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(2, sql.Int64),
),
),
expression.NewOr(
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(3, sql.Int64),
),
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(4, sql.Int64),
),
),
),
true,
},
{
expression.NewOr(
expression.NewIn(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewTuple(
expression.NewLiteral(10, sql.Int64),
expression.NewLiteral(11, sql.Int64),
),
),
expression.NewOr(
expression.NewIn(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewTuple(
expression.NewLiteral(12, sql.Int64),
expression.NewLiteral(13, sql.Int64),
),
),
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(14, sql.Int64),
),
),
),
true,
},
{
expression.NewOr(
expression.NewOr(
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(1, sql.Int64),
),
expression.NewGreaterThan(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(2, sql.Int64),
),
),
expression.NewOr(
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(3, sql.Int64),
),
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(4, sql.Int64),
),
),
),
false,
},
{
expression.NewOr(
expression.NewIn(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewTuple(
expression.NewLiteral(10, sql.Int64),
expression.NewLiteral(11, sql.Int64),
),
),
expression.NewOr(
expression.NewIn(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewTuple(
expression.NewLiteral(12, sql.Int64),
expression.NewLiteral(13, sql.Int64),
),
),
expression.NewGreaterThan(
expression.NewGetFieldWithTable(0, sql.Int64, "foo", "a", false),
expression.NewLiteral(2, sql.Int64),
),
),
),
false,
},
}
schema := sql.Schema{
{Name: "a", Source: "foo"},
Expand All @@ -431,8 +331,6 @@ func TestClassifyFilters(t *testing.T) {
"a": []selector{
selector{1},
selector{5, 6, 7},
selector{1, 2, 3, 4},
selector{10, 11, 12, 13, 14},
},
}, sels)

Expand Down
74 changes: 70 additions & 4 deletions integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,72 @@ func TestIntegration(t *testing.T) {
{"6ecf0ef2c2dffb796033e5a02219af86ec6584e5", int32(9)},
},
},
{
`SELECT commit_hash, file_path
FROM commit_files
WHERE commit_hash='1669dce138d9b841a518c64b10914d88f5e488ea' OR file_path = 'go/example.go'`,
[]sql.Row{
{"e8d3ffab552895c19b9fcf7aa264d277cde33881", "go/example.go"},
{"6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "go/example.go"},
{"918c48b83bd081e863dbe1b80f8998f058cd8294", "go/example.go"},
{"1669dce138d9b841a518c64b10914d88f5e488ea", ".gitignore"},
{"1669dce138d9b841a518c64b10914d88f5e488ea", "CHANGELOG"},
{"1669dce138d9b841a518c64b10914d88f5e488ea", "LICENSE"},
{"1669dce138d9b841a518c64b10914d88f5e488ea", "binary.jpg"},
},
},
{
`SELECT commit_hash, file_path
FROM commit_files
WHERE commit_hash='1669dce138d9b841a518c64b10914d88f5e488ea' AND file_path = 'binary.jpg'`,
[]sql.Row{
{"1669dce138d9b841a518c64b10914d88f5e488ea", "binary.jpg"},
},
},
{
`SELECT commit_hash, file_path
FROM commit_files
WHERE NOT commit_hash = '1669dce138d9b841a518c64b10914d88f5e488ea' AND file_path = 'go/example.go'`,
[]sql.Row{
{"e8d3ffab552895c19b9fcf7aa264d277cde33881", "go/example.go"},
{"6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "go/example.go"},
{"918c48b83bd081e863dbe1b80f8998f058cd8294", "go/example.go"},
},
},
{
`SELECT blob_hash, tree_hash
FROM commit_files
WHERE tree_hash='eba74343e2f15d62adedfd8c883ee0262b5c8021' OR blob_hash = 'd3ff53e0564a9f87d8e84b6e28e5060e517008aa'`,
[]sql.Row{
{"32858aad3c383ed1ff0a0f9bdf231d54a00c9e88", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
{"c192bd6a24ea1ab01d78686e417c8bdc7c3d197f", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
{"d5c0f4ab811897cadf03aec358ae60d21f91c50d", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "a8d315b2b1c615d43042c3a62402b8a54288cf5c"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "fb72698cab7617ac416264415f13224dfd7a165e"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "c2d30fa8ef288618f65f6eed6e168e0d514886f4"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "4d081c50e250fa32ea8b1313cf8bb7c2ad7627fd"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "c2d30fa8ef288618f65f6eed6e168e0d514886f4"},
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "dbd3641b371024f44d0e469a9c8f5457b0660de1"},
},
},
{
`SELECT blob_hash, tree_hash
FROM commit_files
WHERE tree_hash='eba74343e2f15d62adedfd8c883ee0262b5c8021' AND blob_hash = 'd3ff53e0564a9f87d8e84b6e28e5060e517008aa'`,
[]sql.Row{
{"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", "eba74343e2f15d62adedfd8c883ee0262b5c8021"},
},
},
{
`SELECT commit_hash, tree_hash
FROM commits
WHERE commit_author_email='[email protected]' OR commit_hash='b029517f6300c2da0f4b651b8642506cd6aaf45d'`,
[]sql.Row{
{"b029517f6300c2da0f4b651b8642506cd6aaf45d", "aa9b383c260e1d05fbbf6b30a02914555e20c725"},
{"b8e471f58bcbca63b07bda20e428190409c2db47", "c2d30fa8ef288618f65f6eed6e168e0d514886f4"},
},
},
{
`SELECT MONTH(committer_when) as month,
r.repository_id as repo_id,
Expand Down Expand Up @@ -248,10 +314,10 @@ func TestIntegration(t *testing.T) {
SELECT language, COUNT(repository_id) AS repository_count
FROM (
SELECT DISTINCT r.repository_id, LANGUAGE(t.tree_entry_name, b.blob_content) AS language
FROM refs r
JOIN commits c ON r.commit_hash = c.commit_hash
NATURAL JOIN commit_trees
NATURAL JOIN tree_entries t
FROM refs r
JOIN commits c ON r.commit_hash = c.commit_hash
NATURAL JOIN commit_trees
NATURAL JOIN tree_entries t
NATURAL JOIN blobs b
WHERE language IS NOT NULL
) AS q1
Expand Down

0 comments on commit 6c40d3c

Please sign in to comment.