Replace qvalueRange and qvalueRangeMap with analyzeExpr. #2285
Conversation
| rdatum := rcmp.Right.(parser.Datum) | ||
| cmp := ldatum.Compare(rdatum) | ||
|
|
||
| // TODO(pmattis): Figure out how to generate this logic. |
There was a problem hiding this comment.
I wouldn't bother trying to review this switch or the one in simplifyOneOrExpr. Instead, look at the test cases in TestSimplify{And,Or}Expr.
petermattis
force-pushed
the
pmattis/sql-analyze
branch
from
ffca175 to
e48b681
Compare
| if !v.covering { | ||
| return | ||
| } | ||
|
|
||
| v.makeStartInfo(m) | ||
| v.makeEndInfo(m) | ||
| v.makeStartInfo(exprs) |
There was a problem hiding this comment.
both of these methods do nothing if len(exprs) != 1; can analyzeRanges just take a single expr?
There was a problem hiding this comment.
Probably, though I don't want to change this code much in this PR. See the TODO added in one of the commits to planner.selectIndex about where this code is going.
petermattis
force-pushed
the
pmattis/sql-analyze
branch
from
88bde9e to
dea2154
Compare
| }) | ||
|
|
||
| case *parser.AndExpr: | ||
| // De Morgan's Law: (a AND b) -> (NOT a) OR (NOT b) |
There was a problem hiding this comment.
should this have been NOT (a AND b) -> (NOT a) OR (NOT b)?
There was a problem hiding this comment.
Yes, I'm not sure what happened to that NOT.
|
a few outstanding comments and I didn't review the switch as you suggested, but LGTM |
|
I'm going to merge this as-is and fix up the few nits in a separate PR. I already have a few hundred lines of code in a new branch which is based on this and I want to avoid rebase irritations as much as possible. While I don't think it is worthwhile to review the big switches, it would be worthwhile to think about whether there are simpler ways to implement them. Paraphrasing Pascal (Blaise): "I have made this switch longer than usual because I have not had time to make it shorter". |
The previous qvalueRange analysis was limiting and buggy in a variety of ways. It didn't maintain range information in OR expressions in a way that could be used to perform multiple scans of an index for independent ranges.
checkEquivExpr checks that the original and simplified expressions produce identical results for interesting values.
petermattis
force-pushed
the
pmattis/sql-analyze
branch
from
f52fc44 to
0a86f58
Compare
Replace qvalueRange and qvalueRangeMap with analyzeExpr.
Working towards #2140 and #2142.