Skip to content
This repository was archived by the owner on Jan 28, 2021. It is now read-only.

sql/analyzer: add negate index #289

Merged
merged 2 commits into from
Jul 18, 2018

Conversation

mcarmonaa
Copy link
Contributor

Closes #262

Note that multicolumn indexes aren't used for NOT expressions.

@ajnavarro ajnavarro requested review from jfontan and kuba-- July 17, 2018 09:47
case *expression.Not:
r, err := getNegatedIndexes(a, e)
if err != nil {
return result, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not return nil, err?


return getIndexes(or, a)
default:
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that you're returning nil map here and a nil error, that will cause a panic iterating in the previous function over the map (since error is nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iterate over a nil map doesn't cause a panic https://play.golang.org/p/0xG3j4aeS9R

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, you're right. Nvm then

@@ -438,6 +532,7 @@ func getMultiColumnIndexForExpressions(
exprs []columnExpr,
used map[sql.Expression]struct{},
) (index sql.Index, lookup sql.IndexLookup, err error) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty line

Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
@mcarmonaa mcarmonaa force-pushed the improvement/add-negate-index branch from c44a82a to c869740 Compare July 17, 2018 11:58
Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
@mcarmonaa mcarmonaa force-pushed the improvement/add-negate-index branch from c869740 to 60ca3a5 Compare July 17, 2018 11:59
@mcarmonaa
Copy link
Contributor Author

I added a commit handling 'NOT IN' expressions

@ajnavarro ajnavarro merged commit 6c98f53 into src-d:master Jul 18, 2018
@mcarmonaa mcarmonaa deleted the improvement/add-negate-index branch July 19, 2018 07:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants