builtins: new function crdb_internal.ranges_in_span
#97537
Conversation
|
This change is |
| {Name: "start_key", Typ: types.Bytes}, | ||
| {Name: "end_key", Typ: types.Bytes}, |
There was a problem hiding this comment.
Minor feature request: If you add another overload that takes a single parameter that is a byte array and then do something like:
arr := tree.MustBeDArray(args[0])
if arr.Len() != 2 {
return nil, errors.New("expected an array of two elements")
}
startKey := []byte(tree.MustBeDBytes(arr.Array[0]))
endKey := []byte(tree.MustBeDBytes(arr.Array[1]))
Then this function composes well with crdb_internal.tenant_span, crdb_internal.table_span, and crdb_internal.index_span`.
THardy98
force-pushed
the
ranges_in_span
branch
2 times, most recently
from
6c292b0 to
b6756c3
Compare
|
Not sure what's causing the |
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @stevendanna and @THardy98)
pkg/sql/planner.go line 841 at r2 (raw file):
// GetRangeIteratorWithinSpan is part of the eval.Planner interface. func (p *planner) GetRangeIteratorWithinSpan(
I would call this GetRangeDescIterator since the rangedesc.Iterator is different from kvcoord.RangeIterator. You can leave out WithinSpan because that is the only way the rangedesc.Iterator can be created anyways.
pkg/sql/planner.go line 844 at r2 (raw file):
ctx context.Context, span roachpb.Span, ) (rangedesc.Iterator, error) { rangeDescIterator, err := p.execCfg.RangeDescIteratorFactory.NewIterator(ctx, span)
Can simply this to return p.execCfg.RangeDescIteratorFactory.NewIterator(ctx, span) since either rangeDescIterator or err will be nil in the result anyways.
pkg/sql/faketreeeval/evalctx.go line 482 at r2 (raw file):
// GetRangeIteratorWithinSpan is part of the eval.Planner interface. func (ep *DummyEvalPlanner) GetRangeIteratorWithinSpan( ctx context.Context, span roachpb.Span,
These input params don't need names since they are not referenced (see GetRangeDescByID above).
pkg/sql/sem/builtins/generator_builtins.go line 2942 at r2 (raw file):
var err error rs.rangeIter, err = rs.p.GetRangeIteratorWithinSpan(ctx, rs.span) if err != nil {
Similarly here you can just do return err in all cases.
pkg/sql/sem/builtins/generator_builtins.go line 3000 at r2 (raw file):
endKey = []byte(tree.MustBeDBytes(arr.Array[1])) } if len(args) == 2 {
IMO easier to read as
switch len(args) {
case 1:
...
case 2:
...
default:
panic(...)
}
Is it failing when you run it locally? You should be able to debug |
Part of: cockroachdb#96011 This PR introduces a new SRF: `crdb_internal.ranges_in_span(start_key, end_key)` - returns the set of ranges encompassing this span in the form of `(range_id, start_key, end_key)` Release note (sql change): introduce new builtin function `crdb_internal.ranges_in_span(start_key, end_key), returns the set of ranges encompassing the given start and end key.
THardy98
force-pushed
the
ranges_in_span
branch
from
b6756c3 to
1256557
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/planner.go line 841 at r2 (raw file):
Previously, ecwall (Evan Wall) wrote…
I would call this
GetRangeDescIteratorsince therangedesc.Iteratoris different fromkvcoord.RangeIterator. You can leave outWithinSpanbecause that is the only way therangedesc.Iteratorcan be created anyways.
Done
pkg/sql/planner.go line 844 at r2 (raw file):
Previously, ecwall (Evan Wall) wrote…
Can simply this to
return p.execCfg.RangeDescIteratorFactory.NewIterator(ctx, span)since eitherrangeDescIteratororerrwill benilin the result anyways.
Done
pkg/sql/faketreeeval/evalctx.go line 482 at r2 (raw file):
Previously, ecwall (Evan Wall) wrote…
These input params don't need names since they are not referenced (see
GetRangeDescByIDabove).
Done
pkg/sql/sem/builtins/generator_builtins.go line 2942 at r2 (raw file):
Previously, ecwall (Evan Wall) wrote…
Similarly here you can just do
return errin all cases.
Done
pkg/sql/sem/builtins/generator_builtins.go line 3000 at r2 (raw file):
Previously, ecwall (Evan Wall) wrote…
IMO easier to read as
switch len(args) { case 1: ... case 2: ... default: panic(...) }
Agreed, I've switched it to use a switch.
There was a problem hiding this comment.
When I run it via ./dev testlogic --files=ranges_in_span --config=3node-tenant I just seem to be getting a time out:
panic: test timed out after 59m55s
but I don't see what would be causing this.
Reviewable status:
I would step through the new builtin code in the GoLand debugger to figure out where it's getting stuck. If you haven't debugged a logic test in GoLand before, the easiest way is to press the play button and select "Debug" next to the function that was created when you ran |
There was a problem hiding this comment.
I've been running the test via debug and unless I'm missing something (this is my first time doing this), it just seems to run indefinitely.
Reviewable status:
When you are free let's meet up so we can figure this out. |
|
Thomas Hardy seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Part of: #96011
This PR introduces a new SRF:
crdb_internal.ranges_in_span(start_key, end_key)- returns the set of ranges encompassing this span in the form of(range_id, start_key, end_key)Example response:
Release note (sql change): introduce new builtin function `crdb_internal.ranges_in_span(start_key, end_key), returns the set of ranges encompassing the given start and end key.