Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions pkg/util/ranger/ranger.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,11 @@ func UnionRanges(sctx *rangerctx.RangerContext, ranges Ranges, mergeConsecutive
return bytes.Compare(i.encodedStart, j.encodedStart)
})
ranges = ranges[:0]
lastRange := objects[0]
lastRange := &sortRange{
originalValue: objects[0].originalValue.Clone(),
encodedStart: objects[0].encodedStart,
encodedEnd: objects[0].encodedEnd,
}
for i := 1; i < len(objects); i++ {
if (mergeConsecutive && bytes.Compare(lastRange.encodedEnd, objects[i].encodedStart) >= 0) ||
(!mergeConsecutive && bytes.Compare(lastRange.encodedEnd, objects[i].encodedStart) > 0) {
Expand All @@ -692,7 +696,9 @@ func UnionRanges(sctx *rangerctx.RangerContext, ranges Ranges, mergeConsecutive
}
} else {
ranges = append(ranges, lastRange.originalValue)
lastRange = objects[i]
lastRange.encodedStart = objects[i].encodedStart
lastRange.encodedEnd = objects[i].encodedEnd
lastRange.originalValue = objects[i].originalValue.Clone()
Comment on lines +699 to +701

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

After Clone(), lastRange.originalValue is already an independent Range object. The HighVal assignment on line 694 only changes where this cloned object's HighVal field points — it does not mutate the caller's original Range in any way.

While the cloned HighVal and objects[i].HighVal do share the same underlying array, the returned ranges' HighVal is only ever read (for range endpoint comparison), never mutated in-place by downstream consumers. So this slice aliasing is safe in practice.

I'm leaning toward keeping the current approach — it avoids an unnecessary copy and the semantics are correct for this use case. That said, if you feel a deep copy here adds more confidence, I'm open to it.

}
}
ranges = append(ranges, lastRange.originalValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,22 @@ IndexMerge 5555.56 root type: union
├─IndexRangeScan(Build) 3333.33 cop[tikv] table:tt, index:idx1(a) range:(10,+inf], keep order:false, stats:pseudo
├─IndexRangeScan(Build) 3333.33 cop[tikv] table:tt, index:idx2(b) range:(10,+inf], keep order:false, stats:pseudo
└─TableRowIDScan(Probe) 5555.56 cop[tikv] table:tt keep order:false, stats:pseudo
drop table if exists t3;
create table t3(id int primary key, a int not null, b int not null, index pi(b) where a < 3);
insert into t3 values (1, 0, 1), (2, 1, 2), (3, 2, 3), (4, 3, 4), (5, 10, 5);
select id, a, b from t3 where a >= 0 order by b limit 5;
id a b
1 0 1
2 1 2
3 2 3
4 3 4
5 10 5
select id, a, b from t3 ignore index(pi) where a >= 0 order by b limit 5;
id a b
1 0 1
2 1 2
3 2 3
4 3 4
5 10 5
drop table t3;
drop table t, tt;
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,13 @@ explain format='brief' select /*+ inl_join(t2) */ * from tt t1, tt t2 where t1.b
explain format='brief' select /*+ use_index_merge(t, idx1, idx2) */ * from t where (a > 10) or (b > 10);
explain format='brief' select /*+ use_index_merge(tt, idx1, idx2) */ * from tt where (a > 10) or (b > 10);

# TestPartialIndexImplicationCheck
# a >= 0 does NOT imply a < 3, partial index pi should NOT be used.
drop table if exists t3;
create table t3(id int primary key, a int not null, b int not null, index pi(b) where a < 3);
insert into t3 values (1, 0, 1), (2, 1, 2), (3, 2, 3), (4, 3, 4), (5, 10, 5);
select id, a, b from t3 where a >= 0 order by b limit 5;
select id, a, b from t3 ignore index(pi) where a >= 0 order by b limit 5;
drop table t3;

drop table t, tt;
Loading