From efea9ac0e11ba4be45fd6480ca261ba56660bfc5 Mon Sep 17 00:00:00 2001 From: fengttt Date: Fri, 10 Jul 2026 23:36:53 -0700 Subject: [PATCH 1/6] perf: replace stdlib sort.Slice with slices pkg to eliminate heap escapes sort.Slice(s, less) boxes its slice argument into interface{}, which forces `s escapes to heap` (a per-call allocation, confirmed via -gcflags=-m) and uses reflection for element swaps. The generic slices.Sort / slices.SortFunc / slices.SortStableFunc do neither: the slice does not escape and there is no reflection. Convert ~104 stdlib sort.Slice/sort.SliceStable sites across 66 files: - ordered element slices -> slices.Sort - keyed/desc/multi-key -> slices.SortFunc + cmp.Compare (cmp.Compare, never a-b, is overflow-safe) - sort.SliceStable -> slices.SortStableFunc (stability preserved) - index-permutation sorts -> element values index the target array - boolean lessFn predicates -> two-call 3-way comparator Order-sensitive sites: two comparators were NON-STRICT (true on equal) -- the filter-cost sort in stats.go (`cost1 <= cost2`) and the soft-deleted object sort in txnimpl/table.go (`CreatedAt.LE`). Their evaluation order is semantically significant (it selects which predicate a vector/IVF index probes with and which value a runtime error cites), so converting them to a strict cmp.Compare/Compare (0 on ties) would change results (operator/is_not_operator, vector/vector_ivf_mode). These two keep slices.SortFunc but use a comparator that reproduces the original non-strict ordering exactly, so behavior is unchanged. Deliberately left unchanged: - sort.Sort on pointer receivers (a pointer fits in the interface word, so no allocation occurs). - logservicedriver SkipCmd.Sort: its comparator has a side effect (swaps a parallel psns array during comparison). Tracked in #25615. - sort.Search / sort.Strings / sort.Float64s: not slice-boxing. Verified: go build ./..., go vet on all touched packages, gofmt, and BVT (operator, vector, optimizer, join, dml, ...) all pass; behavior is identical to sort.Slice (no golden changes). Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/mo-service/debug.go | 21 +++++----- pkg/backup/tae.go | 12 ++++-- pkg/backup/types.go | 7 ++-- pkg/clusterservice/cluster.go | 7 ++-- pkg/fileservice/aws_sdk_v2.go | 7 ++-- pkg/fileservice/local_etl_fs.go | 7 ++-- pkg/fileservice/local_fs.go | 7 ++-- pkg/fileservice/memory_fs.go | 7 ++-- pkg/fileservice/qcloud_sdk.go | 7 ++-- pkg/fileservice/s3_fs.go | 7 ++-- pkg/frontend/compiler_context.go | 6 +-- pkg/frontend/data_branch_privilege.go | 6 +-- pkg/frontend/logservice.go | 42 ++++++++++--------- pkg/frontend/mysql_cmd_executor.go | 10 ++--- pkg/frontend/rewrite_rule.go | 23 +++++----- pkg/fulltext/fixedbytepool.go | 6 +-- pkg/geo/hull.go | 13 +++--- pkg/hakeeper/bootstrap/bootstrap.go | 23 +++++----- pkg/hakeeper/checkers/logservice/check.go | 19 +++++---- pkg/hakeeper/checkers/logservice/filter.go | 7 ++-- pkg/hakeeper/checkers/logservice/parse.go | 4 +- pkg/hakeeper/checkers/tnservice/check.go | 19 +++++---- pkg/hakeeper/checkers/tnservice/parse.go | 6 +-- pkg/hakeeper/operator/builder.go | 10 ++--- pkg/shardservice/runtime.go | 7 ++-- pkg/shardservice/scheduler_balance.go | 9 ++-- pkg/sql/colexec/external/hive_partition.go | 11 ++--- .../colexec/external/hive_partition_cache.go | 7 ++-- pkg/sql/colexec/lockop/lock_op.go | 6 +-- pkg/sql/colexec/table_function/processlist.go | 6 +-- pkg/sql/compile/compile.go | 37 ++++++++-------- pkg/sql/plan/explain/explain_node.go | 10 ++--- pkg/sql/plan/function/func_binary.go | 10 +++-- pkg/sql/plan/function/func_builtin_jq.go | 7 ++-- pkg/sql/plan/join_order.go | 22 ++++++++-- pkg/sql/plan/stats.go | 20 ++++++--- pkg/sql/plan/tools/helper.go | 6 ++- pkg/sql/schedule/placement.go | 9 ++-- pkg/taskservice/mem_task_storage.go | 21 +++++----- pkg/taskservice/task_runner.go | 6 +-- pkg/tools/checkpointtool/checkpoint_reader.go | 17 ++++---- pkg/txn/client/operator.go | 7 ++-- pkg/txn/storage/memorystorage/mem_handler.go | 11 ++--- pkg/util/debug/goroutine/analyze.go | 7 ++-- pkg/vectorindex/cuvs/cdc.go | 9 ++-- pkg/vm/engine/disttae/cache/catalog.go | 5 ++- .../disttae/local_disttae_datasource.go | 31 ++++++++++---- .../disttae/logtailreplay/get_flush_ts.go | 6 +-- pkg/vm/engine/disttae/tracked_partitions.go | 7 ++-- pkg/vm/engine/disttae/txn.go | 11 +++-- pkg/vm/engine/memoryengine/shard.go | 7 ++-- pkg/vm/engine/memoryengine/shard_hash.go | 15 +++---- pkg/vm/engine/readutil/pk_filter.go | 4 +- pkg/vm/engine/readutil/tombstone_data.go | 5 ++- pkg/vm/engine/tae/catalog/schema.go | 4 +- pkg/vm/engine/tae/common/intervals.go | 11 +++-- pkg/vm/engine/tae/compute/compute.go | 12 ++++-- pkg/vm/engine/tae/db/checkpoint/replay.go | 10 ++--- pkg/vm/engine/tae/db/checkpoint/snapshot.go | 18 ++++---- pkg/vm/engine/tae/db/merge/statLayerZero.go | 10 ++--- pkg/vm/engine/tae/db/merge/statOverlap.go | 9 ++-- .../logstore/driver/batchstoredriver/file.go | 7 ++-- pkg/vm/engine/tae/logtail/ckp_reader.go | 11 ++--- pkg/vm/engine/tae/logtail/handle.go | 4 +- pkg/vm/engine/tae/logtail/snapshot.go | 27 ++++++------ pkg/vm/engine/tae/txn/txnimpl/table.go | 13 ++++-- 66 files changed, 424 insertions(+), 331 deletions(-) diff --git a/cmd/mo-service/debug.go b/cmd/mo-service/debug.go index aeb653d39f2e3..8882daa4f5a10 100644 --- a/cmd/mo-service/debug.go +++ b/cmd/mo-service/debug.go @@ -16,6 +16,7 @@ package main import ( "bytes" + "cmp" "compress/gzip" "context" "flag" @@ -28,7 +29,7 @@ import ( "path" "runtime" "runtime/pprof" - "sort" + "slices" "strings" "time" "unsafe" @@ -175,9 +176,7 @@ func init() { allInfos = append(allInfos, infos) } - sort.Slice(allInfos, func(i, j int) bool { - aInfos := allInfos[i] - bInfos := allInfos[j] + slices.SortFunc(allInfos, func(aInfos, bInfos []positionInfo) int { aNumMOFrame := 0 for _, info := range aInfos { if strings.Contains(info.PackagePath, "matrixone") || @@ -193,14 +192,14 @@ func init() { } } if aNumMOFrame != bNumMOFrame { - return aNumMOFrame > bNumMOFrame + return cmp.Compare(bNumMOFrame, aNumMOFrame) } a := aInfos[len(aInfos)-1] b := bInfos[len(bInfos)-1] if a.FileOnDisk != b.FileOnDisk { - return a.FileOnDisk < b.FileOnDisk + return cmp.Compare(a.FileOnDisk, b.FileOnDisk) } - return a.Line < b.Line + return cmp.Compare(a.Line, b.Line) }) for i, infos := range allInfos { @@ -253,10 +252,10 @@ func init() { } liveBytes[sum] += record.AllocBytes - record.FreeBytes } - sort.Slice(positions, func(i, j int) bool { - _, sum1 := getStackInfo(positions[i][:]) - _, sum2 := getStackInfo(positions[j][:]) - return liveBytes[sum1] > liveBytes[sum2] + slices.SortFunc(positions, func(a, b position) int { + _, sum1 := getStackInfo(a[:]) + _, sum2 := getStackInfo(b[:]) + return cmp.Compare(liveBytes[sum2], liveBytes[sum1]) }) for i, pos := range positions { diff --git a/pkg/backup/tae.go b/pkg/backup/tae.go index d57613df8aa37..a48820546b2a7 100644 --- a/pkg/backup/tae.go +++ b/pkg/backup/tae.go @@ -23,7 +23,7 @@ import ( "os" "path" runtime2 "runtime" - "sort" + "slices" "strconv" "strings" "sync" @@ -556,8 +556,14 @@ func copyFileAndGetMetaFiles( if len(metaFiles) == 0 { return taeFileList, metaFiles, mFiles, nil } - sort.Slice(metaFiles, func(i, j int) bool { - return metaFiles[i].GetEnd().LT(metaFiles[j].GetEnd()) + slices.SortFunc(metaFiles, func(a, b ioutil.TSRangeFile) int { + if a.GetEnd().LT(b.GetEnd()) { + return -1 + } + if b.GetEnd().LT(a.GetEnd()) { + return 1 + } + return 0 }) return taeFileList, metaFiles, mFiles, nil diff --git a/pkg/backup/types.go b/pkg/backup/types.go index e69842fcac195..f7483b6ea8f2a 100644 --- a/pkg/backup/types.go +++ b/pkg/backup/types.go @@ -15,8 +15,9 @@ package backup import ( + "cmp" "fmt" - "sort" + "slices" "strings" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -153,8 +154,8 @@ func (m *Metas) orderTypes() []int { for i := range m.metas { idx = append(idx, i) } - sort.Slice(idx, func(i, j int) bool { - return m.metas[idx[i]].Typ < m.metas[idx[j]].Typ + slices.SortFunc(idx, func(a, b int) int { + return cmp.Compare(m.metas[a].Typ, m.metas[b].Typ) }) return idx } diff --git a/pkg/clusterservice/cluster.go b/pkg/clusterservice/cluster.go index bd6e607cb284e..5f679aa359ee7 100644 --- a/pkg/clusterservice/cluster.go +++ b/pkg/clusterservice/cluster.go @@ -15,8 +15,9 @@ package clusterservice import ( + "cmp" "context" - "sort" + "slices" "sync" "sync/atomic" "time" @@ -372,8 +373,8 @@ func (c *cluster) refresh() { } } // sort as the tick, with the bigger one at the front. - sort.Slice(details.TNStores, func(i, j int) bool { - return details.TNStores[i].Tick > details.TNStores[j].Tick + slices.SortFunc(details.TNStores, func(a, b logpb.TNStore) int { + return cmp.Compare(b.Tick, a.Tick) }) for _, tn := range details.TNStores { v := newTNService(tn) diff --git a/pkg/fileservice/aws_sdk_v2.go b/pkg/fileservice/aws_sdk_v2.go index 37ab5df3753b0..d4a3c0b6ae332 100644 --- a/pkg/fileservice/aws_sdk_v2.go +++ b/pkg/fileservice/aws_sdk_v2.go @@ -16,6 +16,7 @@ package fileservice import ( "bytes" + "cmp" "context" "errors" "fmt" @@ -23,7 +24,7 @@ import ( "iter" "math" gotrace "runtime/trace" - "sort" + "slices" "strings" "sync" "sync/atomic" @@ -711,8 +712,8 @@ func (a *AwsSDKv2) WriteMultipartParallel( return moerr.NewInternalErrorNoCtxf("multipart upload incomplete, expect %d parts got %d", partNum, len(parts)) } - sort.Slice(parts, func(i, j int) bool { - return *parts[i].PartNumber < *parts[j].PartNumber + slices.SortFunc(parts, func(a, b types.CompletedPart) int { + return cmp.Compare(*a.PartNumber, *b.PartNumber) }) _, err = DoWithRetry("complete multipart upload", func() (*s3.CompleteMultipartUploadOutput, error) { diff --git a/pkg/fileservice/local_etl_fs.go b/pkg/fileservice/local_etl_fs.go index 366f8aa171a08..47dc5195027e2 100644 --- a/pkg/fileservice/local_etl_fs.go +++ b/pkg/fileservice/local_etl_fs.go @@ -16,13 +16,14 @@ package fileservice import ( "bytes" + "cmp" "context" "io" "iter" "os" pathpkg "path" "path/filepath" - "sort" + "slices" "strings" "sync" "sync/atomic" @@ -123,8 +124,8 @@ func (l *LocalETLFS) write(ctx context.Context, vector IOVector) error { nativePath := l.toNativeFilePath(path.File) // sort - sort.Slice(vector.Entries, func(i, j int) bool { - return vector.Entries[i].Offset < vector.Entries[j].Offset + slices.SortFunc(vector.Entries, func(a, b IOEntry) int { + return cmp.Compare(a.Offset, b.Offset) }) // size diff --git a/pkg/fileservice/local_fs.go b/pkg/fileservice/local_fs.go index 441dd53b31ec1..ab6c0289aaee8 100644 --- a/pkg/fileservice/local_fs.go +++ b/pkg/fileservice/local_fs.go @@ -16,6 +16,7 @@ package fileservice import ( "bytes" + "cmp" "context" "encoding/binary" "errors" @@ -26,7 +27,7 @@ import ( "os" pathpkg "path" "path/filepath" - "sort" + "slices" "strings" "sync" "sync/atomic" @@ -368,8 +369,8 @@ func (l *LocalFS) write(ctx context.Context, vector IOVector) (bytesWritten int, nativePath := l.toNativeFilePath(path.File) // sort - sort.Slice(vector.Entries, func(i, j int) bool { - return vector.Entries[i].Offset < vector.Entries[j].Offset + slices.SortFunc(vector.Entries, func(a, b IOEntry) int { + return cmp.Compare(a.Offset, b.Offset) }) // size diff --git a/pkg/fileservice/memory_fs.go b/pkg/fileservice/memory_fs.go index 946060b91c191..58ff53b0fb233 100644 --- a/pkg/fileservice/memory_fs.go +++ b/pkg/fileservice/memory_fs.go @@ -16,11 +16,12 @@ package fileservice import ( "bytes" + "cmp" "context" "io" "iter" pathpkg "path" - "sort" + "slices" "strings" "sync" @@ -168,8 +169,8 @@ func (m *MemoryFS) write(ctx context.Context, vector IOVector) error { } } - sort.Slice(vector.Entries, func(i, j int) bool { - return vector.Entries[i].Offset < vector.Entries[j].Offset + slices.SortFunc(vector.Entries, func(a, b IOEntry) int { + return cmp.Compare(a.Offset, b.Offset) }) r := newIOEntriesReader(ctx, vector.Entries) diff --git a/pkg/fileservice/qcloud_sdk.go b/pkg/fileservice/qcloud_sdk.go index 687788eb98f4d..5568dc9df6203 100644 --- a/pkg/fileservice/qcloud_sdk.go +++ b/pkg/fileservice/qcloud_sdk.go @@ -16,6 +16,7 @@ package fileservice import ( "bytes" + "cmp" "context" "errors" "fmt" @@ -25,7 +26,7 @@ import ( "net/url" "os" gotrace "runtime/trace" - "sort" + "slices" "strconv" "sync" "time" @@ -565,8 +566,8 @@ func (a *QCloudSDK) WriteMultipartParallel( return moerr.NewInternalErrorNoCtxf("multipart upload incomplete, expect %d parts got %d", partNum, len(parts)) } - sort.Slice(parts, func(i, j int) bool { - return parts[i].PartNumber < parts[j].PartNumber + slices.SortFunc(parts, func(a, b cos.Object) int { + return cmp.Compare(a.PartNumber, b.PartNumber) }) completeOpt := &cos.CompleteMultipartUploadOptions{ diff --git a/pkg/fileservice/s3_fs.go b/pkg/fileservice/s3_fs.go index 1a28ba7d52580..f47fca07d5dc8 100644 --- a/pkg/fileservice/s3_fs.go +++ b/pkg/fileservice/s3_fs.go @@ -16,13 +16,14 @@ package fileservice import ( "bytes" + "cmp" "context" "errors" "io" "iter" pathpkg "path" "runtime" - "sort" + "slices" "strings" "sync/atomic" "time" @@ -429,8 +430,8 @@ func (s *S3FS) write(ctx context.Context, vector IOVector) (bytesWritten int, er } // sort - sort.Slice(vector.Entries, func(i, j int) bool { - return vector.Entries[i].Offset < vector.Entries[j].Offset + slices.SortFunc(vector.Entries, func(a, b IOEntry) int { + return cmp.Compare(a.Offset, b.Offset) }) // reader diff --git a/pkg/frontend/compiler_context.go b/pkg/frontend/compiler_context.go index 793029ea2a762..f267244fd77d5 100644 --- a/pkg/frontend/compiler_context.go +++ b/pkg/frontend/compiler_context.go @@ -15,12 +15,12 @@ package frontend import ( + "cmp" "context" "encoding/json" "errors" "fmt" "slices" - "sort" "strconv" "strings" "sync" @@ -729,8 +729,8 @@ func (tcc *TxnCompilerContext) ResolveUdf(name string, args []*plan.Expr) (udf * return nil, err } - sort.Slice(matchedList, func(i, j int) bool { - return matchedList[i].Cost < matchedList[j].Cost + slices.SortFunc(matchedList, func(a, b *MatchUdf) int { + return cmp.Compare(a.Cost, b.Cost) }) minCost := matchedList[0].Cost diff --git a/pkg/frontend/data_branch_privilege.go b/pkg/frontend/data_branch_privilege.go index 3ced1ade8bedd..94076a03735ce 100644 --- a/pkg/frontend/data_branch_privilege.go +++ b/pkg/frontend/data_branch_privilege.go @@ -17,7 +17,7 @@ package frontend import ( "context" "fmt" - "sort" + "slices" "strconv" "strings" @@ -672,7 +672,7 @@ func validateDataBranchDeleteDatabaseTarget( for id := range tableNames { tableIDs = append(tableIDs, id) } - sort.Slice(tableIDs, func(i, j int) bool { return tableIDs[i] < tableIDs[j] }) + slices.Sort(tableIDs) return tableIDs, nil } @@ -731,7 +731,7 @@ func validateActiveBranchChildTableIDs( for id := range tableNames { idList = append(idList, id) } - sort.Slice(idList, func(i, j int) bool { return idList[i] < idList[j] }) + slices.Sort(idList) sysCtx := defines.AttachAccountId(ctx, sysAccountID) active := make(map[uint64]struct{}, len(tableNames)) diff --git a/pkg/frontend/logservice.go b/pkg/frontend/logservice.go index ccdd49fef68fd..a37e1ee5b52e0 100644 --- a/pkg/frontend/logservice.go +++ b/pkg/frontend/logservice.go @@ -15,9 +15,10 @@ package frontend import ( + "cmp" "context" "fmt" - "sort" + "slices" "strconv" "github.com/matrixorigin/matrixone/pkg/common/moerr" @@ -209,25 +210,28 @@ func handleShowLogserviceReplicas(execCtx *ExecCtx, ses *Session) error { } func sortLogserviceReplicasResult(infoList []logserviceReplicaInfo) []logserviceReplicaInfo { - sort.Slice(infoList, func(i, j int) bool { - if !infoList[i].hasReplica { - return false + slices.SortFunc(infoList, func(a, b logserviceReplicaInfo) int { + if !a.hasReplica && !b.hasReplica { + return 0 } - if !infoList[j].hasReplica { - return true + if !a.hasReplica { + return 1 } - if infoList[i].shardID != infoList[j].shardID { - return infoList[i].shardID < infoList[j].shardID + if !b.hasReplica { + return -1 } - if infoList[i].replicaRole != infoList[j].replicaRole { - if infoList[i].replicaRole == Leader { - return true - } else if infoList[j].replicaRole == Leader { - return false + if a.shardID != b.shardID { + return cmp.Compare(a.shardID, b.shardID) + } + if a.replicaRole != b.replicaRole { + if a.replicaRole == Leader { + return -1 + } else if b.replicaRole == Leader { + return 1 } - return infoList[i].replicaRole < infoList[j].replicaRole + return cmp.Compare(a.replicaRole, b.replicaRole) } - return infoList[i].replicaID < infoList[j].replicaID + return cmp.Compare(a.replicaID, b.replicaID) }) return infoList } @@ -281,8 +285,8 @@ type replicas []replica func (rs replicas) String() string { var str string l := len(rs) - sort.Slice(rs, func(i, j int) bool { - return rs[i].shardID < rs[j].shardID + slices.SortFunc(rs, func(a, b replica) int { + return cmp.Compare(a.shardID, b.shardID) }) for i, r := range rs { str += r.String() @@ -326,8 +330,8 @@ func handleShowLogserviceStores(execCtx *ExecCtx, ses *Session) error { } func sortLogserviceStoresResult(infoList []logserviceStoreInfo) []logserviceStoreInfo { - sort.Slice(infoList, func(i, j int) bool { - return infoList[i].storeID < infoList[j].storeID + slices.SortFunc(infoList, func(a, b logserviceStoreInfo) int { + return cmp.Compare(a.storeID, b.storeID) }) return infoList } diff --git a/pkg/frontend/mysql_cmd_executor.go b/pkg/frontend/mysql_cmd_executor.go index 7a62fd5842929..35de6b4242921 100644 --- a/pkg/frontend/mysql_cmd_executor.go +++ b/pkg/frontend/mysql_cmd_executor.go @@ -17,6 +17,7 @@ package frontend import ( "bufio" "bytes" + "cmp" "context" "encoding/binary" "encoding/hex" @@ -29,7 +30,6 @@ import ( "reflect" gotrace "runtime/trace" "slices" - "sort" "strconv" "strings" "sync" @@ -1146,8 +1146,8 @@ func doShowVariables(ses *Session, execCtx *ExecCtx, sv *tree.ShowVariables) err } //sort by name - sort.Slice(rows, func(i, j int) bool { - return rows[i][0].(string) < rows[j][0].(string) + slices.SortFunc(rows, func(a, b []interface{}) int { + return cmp.Compare(a[0].(string), b[0].(string)) }) for _, row := range rows { @@ -1942,8 +1942,8 @@ func doShowCollation(ses *Session, execCtx *ExecCtx, proc *process.Process, sc * } //sort by name - sort.Slice(rows, func(i, j int) bool { - return rows[i][0].(string) < rows[j][0].(string) + slices.SortFunc(rows, func(a, b []interface{}) int { + return cmp.Compare(a[0].(string), b[0].(string)) }) for _, row := range rows { diff --git a/pkg/frontend/rewrite_rule.go b/pkg/frontend/rewrite_rule.go index 2256fe8ed9081..f421a07e28da8 100644 --- a/pkg/frontend/rewrite_rule.go +++ b/pkg/frontend/rewrite_rule.go @@ -16,10 +16,11 @@ package frontend import ( "bytes" + "cmp" "context" "encoding/json" "fmt" - "sort" + "slices" "strconv" "strings" @@ -519,19 +520,19 @@ func loadRuleCache(ctx context.Context, ses *Session) (map[string]string, error) // primary role, directly granted secondary roles by grant time, then inherited // roles in breadth-first grant-time order. Later rows override incompatible // earlier rows for the same rule_name. - sort.SliceStable(ruleRows, func(i, j int) bool { - iPriority, iOK := rolePriority[ruleRows[i].roleID] - jPriority, jOK := rolePriority[ruleRows[j].roleID] - if !iOK { - iPriority = len(rolePriority) + slices.SortStableFunc(ruleRows, func(a, b rewriteRuleRow) int { + aPriority, aOK := rolePriority[a.roleID] + bPriority, bOK := rolePriority[b.roleID] + if !aOK { + aPriority = len(rolePriority) } - if !jOK { - jPriority = len(rolePriority) + if !bOK { + bPriority = len(rolePriority) } - if iPriority != jPriority { - return iPriority < jPriority + if aPriority != bPriority { + return cmp.Compare(aPriority, bPriority) } - return ruleRows[i].ruleName < ruleRows[j].ruleName + return cmp.Compare(a.ruleName, b.ruleName) }) for _, row := range ruleRows { diff --git a/pkg/fulltext/fixedbytepool.go b/pkg/fulltext/fixedbytepool.go index 63b9c05ccf2e1..2de2f83814230 100644 --- a/pkg/fulltext/fixedbytepool.go +++ b/pkg/fulltext/fixedbytepool.go @@ -17,7 +17,7 @@ package fulltext import ( "fmt" "os" - "sort" + "slices" "sync" "time" @@ -400,8 +400,8 @@ func (pool *FixedBytePool) Spill() error { return nil } - sort.Slice(lru, func(i, j int) bool { - return lru[i].last_update.Before(lru[j].last_update) + slices.SortFunc(lru, func(a, b Lru) int { + return a.last_update.Compare(b.last_update) }) //fmt.Printf("sorted %v\n", lru) diff --git a/pkg/geo/hull.go b/pkg/geo/hull.go index bb15b88d5b8b5..f85d2eb763e3f 100644 --- a/pkg/geo/hull.go +++ b/pkg/geo/hull.go @@ -14,7 +14,10 @@ package geo -import "sort" +import ( + "cmp" + "slices" +) // ConvexHull returns the convex hull of every coordinate in g: a Polygon when // the points span an area, a LineString when they are collinear (2+ distinct @@ -47,11 +50,11 @@ func monotoneChain(pts []Coord) []Coord { } sorted := make([]Coord, len(pts)) copy(sorted, pts) - sort.Slice(sorted, func(i, j int) bool { - if sorted[i].X != sorted[j].X { - return sorted[i].X < sorted[j].X + slices.SortFunc(sorted, func(a, b Coord) int { + if a.X != b.X { + return cmp.Compare(a.X, b.X) } - return sorted[i].Y < sorted[j].Y + return cmp.Compare(a.Y, b.Y) }) uniq := sorted[:1] for _, p := range sorted[1:] { diff --git a/pkg/hakeeper/bootstrap/bootstrap.go b/pkg/hakeeper/bootstrap/bootstrap.go index cdbc717dfa44a..3dbbd09579c75 100644 --- a/pkg/hakeeper/bootstrap/bootstrap.go +++ b/pkg/hakeeper/bootstrap/bootstrap.go @@ -15,7 +15,8 @@ package bootstrap import ( - "sort" + "cmp" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/runtime" @@ -164,18 +165,18 @@ func logStoresSortedByTick(logStores map[string]pb.LogStoreInfo) []string { uuidSlice = append(uuidSlice, uuid) } - sort.Slice(uuidSlice, func(i, j int) bool { + slices.SortFunc(uuidSlice, func(a, b string) int { // FIXME(volgariver6): currently, the locality of log stores should be empty, // but it could not be empty actually. - if len(logStores[uuidSlice[i]].Locality.Value) == 0 && - len(logStores[uuidSlice[j]].Locality.Value) > 0 { - return true + if len(logStores[a].Locality.Value) == 0 && + len(logStores[b].Locality.Value) > 0 { + return -1 } - if len(logStores[uuidSlice[i]].Locality.Value) > 0 && - len(logStores[uuidSlice[j]].Locality.Value) == 0 { - return false + if len(logStores[a].Locality.Value) > 0 && + len(logStores[b].Locality.Value) == 0 { + return 1 } - return logStores[uuidSlice[i]].Tick > logStores[uuidSlice[j]].Tick + return cmp.Compare(logStores[b].Tick, logStores[a].Tick) }) return uuidSlice @@ -187,8 +188,8 @@ func tnStoresSortedByTick(tnStores map[string]pb.TNStoreInfo) []string { uuidSlice = append(uuidSlice, uuid) } - sort.Slice(uuidSlice, func(i, j int) bool { - return tnStores[uuidSlice[i]].Tick > tnStores[uuidSlice[j]].Tick + slices.SortFunc(uuidSlice, func(a, b string) int { + return cmp.Compare(tnStores[b].Tick, tnStores[a].Tick) }) return uuidSlice diff --git a/pkg/hakeeper/checkers/logservice/check.go b/pkg/hakeeper/checkers/logservice/check.go index ec30320149496..3d319fac67d2a 100644 --- a/pkg/hakeeper/checkers/logservice/check.go +++ b/pkg/hakeeper/checkers/logservice/check.go @@ -15,8 +15,9 @@ package logservice import ( + "cmp" "fmt" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/runtime" "github.com/matrixorigin/matrixone/pkg/hakeeper" @@ -421,20 +422,20 @@ func (t *checkAddShard) check() []*operator.Operator { stores = append(stores, store) } - sort.Slice(stores, func(i, j int) bool { - _, ok1 := t.lc.logState.Stores[stores[i]] - _, ok2 := t.lc.logState.Stores[stores[j]] + slices.SortFunc(stores, func(a, b string) int { + _, ok1 := t.lc.logState.Stores[a] + _, ok2 := t.lc.logState.Stores[b] if !ok1 && !ok2 { - return false + return 0 } if !ok1 && ok2 { - return false + return 1 } if ok1 && !ok2 { - return true + return -1 } - return t.lc.logState.Stores[stores[i]].Tick > - t.lc.logState.Stores[stores[j]].Tick + return cmp.Compare(t.lc.logState.Stores[b].Tick, + t.lc.logState.Stores[a].Tick) }) var maxShardID uint64 diff --git a/pkg/hakeeper/checkers/logservice/filter.go b/pkg/hakeeper/checkers/logservice/filter.go index eb61bc0dbc993..9fbb9d59b4e97 100644 --- a/pkg/hakeeper/checkers/logservice/filter.go +++ b/pkg/hakeeper/checkers/logservice/filter.go @@ -15,7 +15,8 @@ package logservice import ( - "sort" + "cmp" + "slices" "github.com/matrixorigin/matrixone/pkg/hakeeper/checkers/util" "github.com/matrixorigin/matrixone/pkg/pb/logservice" @@ -56,8 +57,8 @@ func selectStore( return "" } - sort.Slice(candidates, func(i, j int) bool { - return candidates[i].ID < candidates[j].ID + slices.SortFunc(candidates, func(a, b *util.Store) int { + return cmp.Compare(a.ID, b.ID) }) return candidates[0].ID diff --git a/pkg/hakeeper/checkers/logservice/parse.go b/pkg/hakeeper/checkers/logservice/parse.go index cecc13a163c4d..2101191127018 100644 --- a/pkg/hakeeper/checkers/logservice/parse.go +++ b/pkg/hakeeper/checkers/logservice/parse.go @@ -15,7 +15,7 @@ package logservice import ( - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/hakeeper" pb "github.com/matrixorigin/matrixone/pkg/pb/logservice" @@ -275,7 +275,7 @@ func sortedReplicaID(replicas map[uint64]string, leaderID uint64) []uint64 { exist = true } } - sort.Slice(idSlice, func(i, j int) bool { return idSlice[i] < idSlice[j] }) + slices.Sort(idSlice) if exist { idSlice = append(idSlice, leaderID) } diff --git a/pkg/hakeeper/checkers/tnservice/check.go b/pkg/hakeeper/checkers/tnservice/check.go index 74e2a9fdfdea4..e40e584d2a186 100644 --- a/pkg/hakeeper/checkers/tnservice/check.go +++ b/pkg/hakeeper/checkers/tnservice/check.go @@ -15,7 +15,8 @@ package tnservice import ( - "sort" + "cmp" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/runtime" @@ -209,8 +210,8 @@ func newRemoveStep(target string, shardID, replicaID, logShardID uint64) operato func expiredReplicas(shard *tnShard) []*tnReplica { expired := shard.expiredReplicas() // less replica first - sort.Slice(expired, func(i, j int) bool { - return expired[i].replicaID < expired[j].replicaID + slices.SortFunc(expired, func(a, b *tnReplica) int { + return cmp.Compare(a.replicaID, b.replicaID) }) return expired } @@ -224,8 +225,8 @@ func extraWorkingReplicas(shard *tnShard) []*tnReplica { } // less replica first - sort.Slice(working, func(i, j int) bool { - return working[i].replicaID < working[j].replicaID + slices.SortFunc(working, func(a, b *tnReplica) int { + return cmp.Compare(a.replicaID, b.replicaID) }) return working[0 : len(working)-1] @@ -241,8 +242,8 @@ func consumeLeastSpareStore(working []*util.Store) (string, error) { } // the least shards, the higher priority - sort.Slice(working, func(i, j int) bool { - return working[i].Length < working[j].Length + slices.SortFunc(working, func(a, b *util.Store) int { + return cmp.Compare(a.Length, b.Length) }) // stores with the same Length @@ -256,8 +257,8 @@ func consumeLeastSpareStore(working []*util.Store) (string, error) { } leastStores = append(leastStores, store) } - sort.Slice(leastStores, func(i, j int) bool { - return leastStores[i].ID < leastStores[j].ID + slices.SortFunc(leastStores, func(a, b *util.Store) int { + return cmp.Compare(a.ID, b.ID) }) // consume a slot from this tn store diff --git a/pkg/hakeeper/checkers/tnservice/parse.go b/pkg/hakeeper/checkers/tnservice/parse.go index 5e45710ba3390..86acedd3276a2 100644 --- a/pkg/hakeeper/checkers/tnservice/parse.go +++ b/pkg/hakeeper/checkers/tnservice/parse.go @@ -16,7 +16,7 @@ package tnservice import ( "fmt" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/runtime" @@ -98,9 +98,7 @@ func checkReportedState( reported := rs.listShards() // keep order of all shards deterministic - sort.Slice(reported, func(i, j int) bool { - return reported[i] < reported[j] - }) + slices.Sort(reported) for _, shardID := range reported { shard, err := rs.getShard(shardID) diff --git a/pkg/hakeeper/operator/builder.go b/pkg/hakeeper/operator/builder.go index 705e51f7d9aba..fbc095a8702df 100644 --- a/pkg/hakeeper/operator/builder.go +++ b/pkg/hakeeper/operator/builder.go @@ -22,7 +22,7 @@ package operator import ( "fmt" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" @@ -232,7 +232,7 @@ func (b *Builder) buildToRemoveSteps() *Builder { for target := range b.targetPeers { targets = append(targets, target) } - sort.Slice(targets, func(i, j int) bool { return targets[i] < targets[j] }) + slices.Sort(targets) uuid, replicaID := b.toRemove.Get() b.steps = append(b.steps, RemoveLogService{ @@ -250,7 +250,7 @@ func (b *Builder) buildToRemoveNonVotingSteps() *Builder { for target := range b.targetPeers { targets = append(targets, target) } - sort.Slice(targets, func(i, j int) bool { return targets[i] < targets[j] }) + slices.Sort(targets) uuid, replicaID := b.toRemoveNonVoting.Get() b.steps = append(b.steps, RemoveNonVotingLogService{ @@ -268,7 +268,7 @@ func (b *Builder) buildToAddSteps() *Builder { for target := range b.originPeers { targets = append(targets, target) } - sort.Slice(targets, func(i, j int) bool { return targets[i] < targets[j] }) + slices.Sort(targets) uuid, replicaID := b.toAdd.Get() b.steps = append(b.steps, AddLogService{ @@ -291,7 +291,7 @@ func (b *Builder) buildToAddNonVotingSteps() *Builder { for target := range b.originPeers { targets = append(targets, target) } - sort.Slice(targets, func(i, j int) bool { return targets[i] < targets[j] }) + slices.Sort(targets) uuid, replicaID := b.toAddNonVoting.Get() b.steps = append(b.steps, AddNonVotingLogService{ diff --git a/pkg/shardservice/runtime.go b/pkg/shardservice/runtime.go index 38c443eb572f7..6fc6c42a282f0 100644 --- a/pkg/shardservice/runtime.go +++ b/pkg/shardservice/runtime.go @@ -15,8 +15,9 @@ package shardservice import ( + "cmp" "fmt" - "sort" + "slices" "sync" "github.com/matrixorigin/matrixone/pkg/common/log" @@ -228,8 +229,8 @@ func (r *rt) filterCN( cns []*cn, filters ...filter, ) []*cn { - sort.Slice(cns, func(i, j int) bool { - return cns[i].id < cns[j].id + slices.SortFunc(cns, func(a, b *cn) int { + return cmp.Compare(a.id, b.id) }) for _, f := range filters { cns = f.filter(r, cns) diff --git a/pkg/shardservice/scheduler_balance.go b/pkg/shardservice/scheduler_balance.go index 61ed9f503681e..e3bd93e52f2a2 100644 --- a/pkg/shardservice/scheduler_balance.go +++ b/pkg/shardservice/scheduler_balance.go @@ -15,7 +15,8 @@ package shardservice import ( - "sort" + "cmp" + "slices" v2 "github.com/matrixorigin/matrixone/pkg/util/metric/v2" "go.uber.org/zap" @@ -155,10 +156,10 @@ func (s *balanceScheduler) doBalance( return false, nil } - sort.Slice( + slices.SortFunc( cns, - func(i, j int) bool { - return t.getReplicaCount(cns[i].id) < t.getReplicaCount(cns[j].id) + func(a, b *cn) int { + return cmp.Compare(t.getReplicaCount(a.id), t.getReplicaCount(b.id)) }, ) diff --git a/pkg/sql/colexec/external/hive_partition.go b/pkg/sql/colexec/external/hive_partition.go index 5ae03c33a0436..dd4bff0b8e129 100644 --- a/pkg/sql/colexec/external/hive_partition.go +++ b/pkg/sql/colexec/external/hive_partition.go @@ -15,12 +15,13 @@ package external import ( + "cmp" "context" "fmt" "iter" "net/url" "path" - "sort" + "slices" "strconv" "strings" "sync" @@ -497,8 +498,8 @@ func DiscoverHivePartitionsWithPruneExpr( if err != nil { return nil, err } - sort.Slice(result.Files, func(i, j int) bool { - return result.Files[i].FilePath < result.Files[j].FilePath + slices.SortFunc(result.Files, func(a, b PartitionFileEntry) int { + return cmp.Compare(a.FilePath, b.FilePath) }) result.DiscoveredFiles = len(result.Files) for _, file := range result.Files { @@ -699,8 +700,8 @@ func discoverRecursive( } } - sort.Slice(childPrefixes, func(i, j int) bool { - return childPrefixes[i].prefix < childPrefixes[j].prefix + slices.SortFunc(childPrefixes, func(a, b childPartition) int { + return cmp.Compare(a.prefix, b.prefix) }) // Count all matching partitions at this level before descending. Otherwise diff --git a/pkg/sql/colexec/external/hive_partition_cache.go b/pkg/sql/colexec/external/hive_partition_cache.go index 31919b9224bf1..4b7f30a63d204 100644 --- a/pkg/sql/colexec/external/hive_partition_cache.go +++ b/pkg/sql/colexec/external/hive_partition_cache.go @@ -15,11 +15,12 @@ package external import ( + "cmp" "context" "crypto/sha256" "encoding/hex" "fmt" - "sort" + "slices" "strings" "sync" "time" @@ -165,8 +166,8 @@ func collectHivePartitionListEntries( } entries = append(entries, *entry) } - sort.Slice(entries, func(i, j int) bool { - return entries[i].Name < entries[j].Name + slices.SortFunc(entries, func(a, b fileservice.DirEntry) int { + return cmp.Compare(a.Name, b.Name) }) return entries, nil } diff --git a/pkg/sql/colexec/lockop/lock_op.go b/pkg/sql/colexec/lockop/lock_op.go index 006502b62bed6..ae0b598186570 100644 --- a/pkg/sql/colexec/lockop/lock_op.go +++ b/pkg/sql/colexec/lockop/lock_op.go @@ -20,7 +20,7 @@ import ( "encoding/hex" "fmt" "math" - "sort" + "slices" "strings" "time" @@ -1448,8 +1448,8 @@ func dedupLockRows(rows [][]byte) [][]byte { if len(rows) <= 1 { return rows } - sort.Slice(rows, func(i, j int) bool { - return bytes.Compare(rows[i], rows[j]) < 0 + slices.SortFunc(rows, func(a, b []byte) int { + return bytes.Compare(a, b) }) deduped := rows[:1] for i := 1; i < len(rows); i++ { diff --git a/pkg/sql/colexec/table_function/processlist.go b/pkg/sql/colexec/table_function/processlist.go index 4b33e89d51460..d28fbdd9911bc 100644 --- a/pkg/sql/colexec/table_function/processlist.go +++ b/pkg/sql/colexec/table_function/processlist.go @@ -16,7 +16,7 @@ package table_function import ( "context" - "sort" + "slices" "strings" "time" @@ -190,8 +190,8 @@ func fetchSessions( retErr = queryservice.RequestMultipleCn(ctx, nodes, qc, genRequest, handleValidResponse, nil) // Sort by session start time. - sort.Slice(sessions, func(i, j int) bool { - return sessions[i].SessionStart.Before(sessions[j].SessionStart) + slices.SortFunc(sessions, func(a, b *status.Session) int { + return a.SessionStart.Compare(b.SessionStart) }) return sessions, retErr diff --git a/pkg/sql/compile/compile.go b/pkg/sql/compile/compile.go index 91285f257d19e..c9ee7c51bc5d7 100644 --- a/pkg/sql/compile/compile.go +++ b/pkg/sql/compile/compile.go @@ -15,12 +15,13 @@ package compile import ( + "cmp" "context" "encoding/hex" "encoding/json" "fmt" "io" - "sort" + "slices" "strconv" "strings" "time" @@ -779,9 +780,7 @@ func (c *Compile) lockTable() error { for tableID := range c.lockTables { tableIDs = append(tableIDs, tableID) } - sort.Slice(tableIDs, func(i, j int) bool { - return tableIDs[i] < tableIDs[j] - }) + slices.Sort(tableIDs) for _, tableID := range tableIDs { tbl := c.lockTables[tableID] typ := plan2.MakeTypeByPlan2Type(tbl.PrimaryColTyp) @@ -2313,8 +2312,8 @@ func splitHiveFileShards(fileList []string, fileSize []int64, nodes []engine.Nod for i := range indices { indices[i] = i } - sort.SliceStable(indices, func(i, j int) bool { - return hiveFileSizeAt(fileSize, indices[i]) > hiveFileSizeAt(fileSize, indices[j]) + slices.SortStableFunc(indices, func(a, b int) int { + return cmp.Compare(hiveFileSizeAt(fileSize, b), hiveFileSizeAt(fileSize, a)) }) for _, fileIdx := range indices { @@ -2364,16 +2363,16 @@ func splitParquetRowGroupShards( for i := range indices { indices[i] = i } - sort.SliceStable(indices, func(i, j int) bool { - left := rowGroups[indices[i]] - right := rowGroups[indices[j]] - if parquetRowGroupLoad(left) != parquetRowGroupLoad(right) { - return parquetRowGroupLoad(left) > parquetRowGroupLoad(right) + slices.SortStableFunc(indices, func(a, b int) int { + left := rowGroups[a] + right := rowGroups[b] + if c := cmp.Compare(parquetRowGroupLoad(right), parquetRowGroupLoad(left)); c != 0 { + return c } - if left.fileIndex != right.fileIndex { - return left.fileIndex < right.fileIndex + if c := cmp.Compare(left.fileIndex, right.fileIndex); c != 0 { + return c } - return left.rowGroupIndex < right.rowGroupIndex + return cmp.Compare(left.rowGroupIndex, right.rowGroupIndex) }) for _, rowGroupIdx := range indices { @@ -2415,13 +2414,11 @@ func splitParquetRowGroupShards( if len(shard.rowGroupShards) == 0 { continue } - sort.SliceStable(shard.rowGroupShards, func(i, j int) bool { - left := shard.rowGroupShards[i] - right := shard.rowGroupShards[j] - if left.FileIndex != right.FileIndex { - return left.FileIndex < right.FileIndex + slices.SortStableFunc(shard.rowGroupShards, func(left, right *pipeline.ParquetRowGroupShard) int { + if c := cmp.Compare(left.FileIndex, right.FileIndex); c != 0 { + return c } - return left.RowGroupStart < right.RowGroupStart + return cmp.Compare(left.RowGroupStart, right.RowGroupStart) }) shard.rowGroupShards = mergeAdjacentParquetRowGroupShards(shard.rowGroupShards) shard.originalToLocal = nil diff --git a/pkg/sql/plan/explain/explain_node.go b/pkg/sql/plan/explain/explain_node.go index 968da6d041570..d1c51a73b85f7 100644 --- a/pkg/sql/plan/explain/explain_node.go +++ b/pkg/sql/plan/explain/explain_node.go @@ -18,7 +18,7 @@ import ( "bytes" "context" "fmt" - "sort" + "slices" "strconv" "github.com/matrixorigin/matrixone/pkg/common" @@ -1121,9 +1121,7 @@ func (a AnalyzeInfoDescribeImpl) GetDescription(ctx context.Context, options *Ex majordop := len(a.AnalyzeInfo.TimeConsumedArrayMajor) if majordop > 1 { fmt.Fprintf(buf, " %v_time=[", majorStr) - sort.Slice(a.AnalyzeInfo.TimeConsumedArrayMajor, func(i, j int) bool { - return a.AnalyzeInfo.TimeConsumedArrayMajor[i] < a.AnalyzeInfo.TimeConsumedArrayMajor[j] - }) + slices.Sort(a.AnalyzeInfo.TimeConsumedArrayMajor) if majordop > 4 { var totalTime int64 for i := range a.AnalyzeInfo.TimeConsumedArrayMajor { @@ -1157,9 +1155,7 @@ func (a AnalyzeInfoDescribeImpl) GetDescription(ctx context.Context, options *Ex } fmt.Fprintf(buf, " %v_time=[", minorStr) - sort.Slice(a.AnalyzeInfo.TimeConsumedArrayMinor, func(i, j int) bool { - return a.AnalyzeInfo.TimeConsumedArrayMinor[i] < a.AnalyzeInfo.TimeConsumedArrayMinor[j] - }) + slices.Sort(a.AnalyzeInfo.TimeConsumedArrayMinor) if minordop > 4 { var totalTime int64 for i := range a.AnalyzeInfo.TimeConsumedArrayMinor { diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 2f05c82a2d978..bff61a74090f1 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -16,6 +16,7 @@ package function import ( "bytes" + "cmp" "context" "crypto/aes" "crypto/cipher" @@ -27,6 +28,7 @@ import ( "math" "math/big" "math/bits" + "slices" "sort" "strconv" "strings" @@ -10483,11 +10485,11 @@ func parameterIntervalsCoverSegment(intervals []geometryParamInterval) bool { return false } - sort.Slice(intervals, func(i, j int) bool { - if sameGeometryCoordinate(intervals[i].start, intervals[j].start) { - return intervals[i].end < intervals[j].end + slices.SortFunc(intervals, func(a, b geometryParamInterval) int { + if sameGeometryCoordinate(a.start, b.start) { + return cmp.Compare(a.end, b.end) } - return intervals[i].start < intervals[j].start + return cmp.Compare(a.start, b.start) }) coveredEnd := intervals[0].end diff --git a/pkg/sql/plan/function/func_builtin_jq.go b/pkg/sql/plan/function/func_builtin_jq.go index 067e6526fe842..abda762df73dc 100644 --- a/pkg/sql/plan/function/func_builtin_jq.go +++ b/pkg/sql/plan/function/func_builtin_jq.go @@ -16,11 +16,12 @@ package function import ( "bytes" + "cmp" "encoding/json" "fmt" "math" "math/big" - "sort" + "slices" "strconv" "unicode/utf8" @@ -436,8 +437,8 @@ func (e *JqEncoder) encodeObject(vs map[string]any) error { kvs[i] = keyVal{k, v} i++ } - sort.Slice(kvs, func(i, j int) bool { - return kvs[i].key < kvs[j].key + slices.SortFunc(kvs, func(a, b keyVal) int { + return cmp.Compare(a.key, b.key) }) for i, kv := range kvs { if i > 0 { diff --git a/pkg/sql/plan/join_order.go b/pkg/sql/plan/join_order.go index 764024c2f2466..02d8b8cd6d854 100644 --- a/pkg/sql/plan/join_order.go +++ b/pkg/sql/plan/join_order.go @@ -16,7 +16,7 @@ package plan import ( "fmt" - "sort" + "slices" "strings" "github.com/matrixorigin/matrixone/pkg/catalog" @@ -246,7 +246,15 @@ func (builder *QueryBuilder) determineJoinOrder(nodeID int32) int32 { } } - sort.Slice(subTrees, func(i, j int) bool { return compareStats(subTrees[i].Stats, subTrees[j].Stats) }) + slices.SortFunc(subTrees, func(a, b *plan.Node) int { + if compareStats(a.Stats, b.Stats) { + return -1 + } + if compareStats(b.Stats, a.Stats) { + return 1 + } + return 0 + }) leafByTag := make(map[int32]int32) @@ -579,7 +587,15 @@ func (builder *QueryBuilder) buildSubJoinTree(vertices []*joinVertex, vid int32) builder.buildSubJoinTree(vertices, child) dimensions = append(dimensions, vertices[child]) } - sort.Slice(dimensions, func(i, j int) bool { return compareStats(dimensions[i].node.Stats, dimensions[j].node.Stats) }) + slices.SortFunc(dimensions, func(a, b *joinVertex) int { + if compareStats(a.node.Stats, b.node.Stats) { + return -1 + } + if compareStats(b.node.Stats, a.node.Stats) { + return 1 + } + return 0 + }) for _, child := range dimensions { diff --git a/pkg/sql/plan/stats.go b/pkg/sql/plan/stats.go index af730ef6f5640..6133f246528de 100644 --- a/pkg/sql/plan/stats.go +++ b/pkg/sql/plan/stats.go @@ -19,7 +19,7 @@ import ( "context" "fmt" "math" - "sort" + "slices" "strconv" "strings" "sync/atomic" @@ -1069,10 +1069,20 @@ func sortFilterListByStats(ctx context.Context, nodeID int32, builder *QueryBuil switch node.NodeType { case plan.Node_TABLE_SCAN: if node.ObjRef != nil && len(node.FilterList) >= 1 { - sort.Slice(node.FilterList, func(i, j int) bool { - cost1 := estimateFilterWeight(node.FilterList[i], 0) * node.FilterList[i].Selectivity - cost2 := estimateFilterWeight(node.FilterList[j], 0) * node.FilterList[j].Selectivity - return cost1 <= cost2 + // Filter evaluation order here is semantically significant: it decides + // which predicate a vector/IVF index probes with and which value a + // runtime error reports (see operator/is_not_operator, + // vector/vector_ivf_mode). The original sort.Slice comparator was + // non-strict (cost1 <= cost2); a strict cmp.Compare (0 on ties) would + // reorder equal-cost filters and change results. Reproduce the exact + // original ordering here while avoiding sort.Slice's interface{} boxing. + slices.SortFunc(node.FilterList, func(a, b *plan.Expr) int { + cost1 := estimateFilterWeight(a, 0) * a.Selectivity + cost2 := estimateFilterWeight(b, 0) * b.Selectivity + if cost1 <= cost2 { + return -1 + } + return 1 }) } } diff --git a/pkg/sql/plan/tools/helper.go b/pkg/sql/plan/tools/helper.go index dcb1457779960..67d343d9d45bd 100644 --- a/pkg/sql/plan/tools/helper.go +++ b/pkg/sql/plan/tools/helper.go @@ -15,6 +15,8 @@ package tools import ( + "cmp" + "slices" "sort" ) @@ -98,8 +100,8 @@ func VarRefsEqual(a, b []VarRef) bool { if alen == 0 { return true } - sort.Slice(a, func(i, j int) bool { return a[i].Name < a[j].Name }) - sort.Slice(b, func(i, j int) bool { return b[i].Name < b[j].Name }) + slices.SortFunc(a, func(x, y VarRef) int { return cmp.Compare(x.Name, y.Name) }) + slices.SortFunc(b, func(x, y VarRef) int { return cmp.Compare(x.Name, y.Name) }) for i := 0; i < alen; i++ { if a[i].Name != b[i].Name { return false diff --git a/pkg/sql/schedule/placement.go b/pkg/sql/schedule/placement.go index 6a6784c53c3f9..c0e941d69199f 100644 --- a/pkg/sql/schedule/placement.go +++ b/pkg/sql/schedule/placement.go @@ -14,7 +14,10 @@ package schedule -import "sort" +import ( + "cmp" + "slices" +) const ( ReasonLocalExecType = "local-exec-type" @@ -184,8 +187,8 @@ func orderDecisionWorkers(req QueryRequest, workers Workers, reason string) Work } func sortWorkersByAddr(workers Workers) { - sort.Slice(workers, func(i, j int) bool { - return workers[i].Addr < workers[j].Addr + slices.SortFunc(workers, func(a, b Worker) int { + return cmp.Compare(a.Addr, b.Addr) }) } diff --git a/pkg/taskservice/mem_task_storage.go b/pkg/taskservice/mem_task_storage.go index 3fc2c7bdaaa33..bd288f94f29bb 100644 --- a/pkg/taskservice/mem_task_storage.go +++ b/pkg/taskservice/mem_task_storage.go @@ -15,9 +15,10 @@ package taskservice import ( + "cmp" "context" "fmt" - "sort" + "slices" "sync" "time" @@ -137,7 +138,7 @@ func (s *memTaskStorage) QueryAsyncTask(ctx context.Context, conds ...Condition) for _, task := range s.asyncTasks { sortedTasks = append(sortedTasks, task) } - sort.Slice(sortedTasks, func(i, j int) bool { return sortedTasks[i].ID < sortedTasks[j].ID }) + slices.SortFunc(sortedTasks, func(a, b task.AsyncTask) int { return cmp.Compare(a.ID, b.ID) }) var result []task.AsyncTask for _, task := range sortedTasks { @@ -177,7 +178,7 @@ func (s *memTaskStorage) QueryCronTask(context.Context, ...Condition) ([]task.Cr for _, v := range s.cronTasks { tasks = append(tasks, v) } - sort.Slice(tasks, func(i, j int) bool { return tasks[i].ID < tasks[j].ID }) + slices.SortFunc(tasks, func(a, b task.CronTask) int { return cmp.Compare(a.ID, b.ID) }) return tasks, nil } @@ -287,7 +288,7 @@ func (s *memTaskStorage) QuerySQLTask(ctx context.Context, conds ...Condition) ( for _, t := range s.sqlTasks { sortedTasks = append(sortedTasks, t) } - sort.Slice(sortedTasks, func(i, j int) bool { return sortedTasks[i].TaskID < sortedTasks[j].TaskID }) + slices.SortFunc(sortedTasks, func(a, b SQLTask) int { return cmp.Compare(a.TaskID, b.TaskID) }) var result []SQLTask for _, t := range sortedTasks { @@ -340,7 +341,7 @@ func (s *memTaskStorage) QuerySQLTaskRun(ctx context.Context, conds ...Condition for _, run := range s.sqlTaskRuns { sortedRuns = append(sortedRuns, run) } - sort.Slice(sortedRuns, func(i, j int) bool { return sortedRuns[i].RunID > sortedRuns[j].RunID }) + slices.SortFunc(sortedRuns, func(a, b SQLTaskRun) int { return cmp.Compare(b.RunID, a.RunID) }) var result []SQLTaskRun for _, run := range sortedRuns { @@ -385,11 +386,11 @@ func (s *memTaskStorage) QueryLatestSQLTaskRun(ctx context.Context, accountID ui for _, run := range latestByTask { result = append(result, run) } - sort.Slice(result, func(i, j int) bool { - if result[i].TaskID == result[j].TaskID { - return result[i].RunID > result[j].RunID + slices.SortFunc(result, func(a, b SQLTaskRun) int { + if c := cmp.Compare(a.TaskID, b.TaskID); c != 0 { + return c } - return result[i].TaskID < result[j].TaskID + return cmp.Compare(b.RunID, a.RunID) }) return result, nil } @@ -498,7 +499,7 @@ func (s *memTaskStorage) QueryDaemonTask(ctx context.Context, conds ...Condition for _, t := range s.daemonTasks { sortedTasks = append(sortedTasks, deepcopy.Copy(t).(task.DaemonTask)) } - sort.Slice(sortedTasks, func(i, j int) bool { return sortedTasks[i].ID < sortedTasks[j].ID }) + slices.SortFunc(sortedTasks, func(a, b task.DaemonTask) int { return cmp.Compare(a.ID, b.ID) }) var result []task.DaemonTask for _, task := range sortedTasks { diff --git a/pkg/taskservice/task_runner.go b/pkg/taskservice/task_runner.go index 9d6a728064f58..f2a5e4b0db330 100644 --- a/pkg/taskservice/task_runner.go +++ b/pkg/taskservice/task_runner.go @@ -17,7 +17,7 @@ package taskservice import ( "context" "runtime" - "sort" + "slices" "sync" "sync/atomic" "time" @@ -554,8 +554,8 @@ func (r *taskRunner) addRetryTask(task runningTask) bool { } r.retryTasks.s = append(r.retryTasks.s, task) - sort.Slice(r.retryTasks.s, func(i, j int) bool { - return r.retryTasks.s[i].retryAt.Before(r.retryTasks.s[j].retryAt) + slices.SortFunc(r.retryTasks.s, func(a, b runningTask) int { + return a.retryAt.Compare(b.retryAt) }) return true } diff --git a/pkg/tools/checkpointtool/checkpoint_reader.go b/pkg/tools/checkpointtool/checkpoint_reader.go index 3f27fb4e15d26..8830b3e483e19 100644 --- a/pkg/tools/checkpointtool/checkpoint_reader.go +++ b/pkg/tools/checkpointtool/checkpoint_reader.go @@ -15,9 +15,10 @@ package checkpointtool import ( + "cmp" "context" "fmt" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/mpool" @@ -123,9 +124,9 @@ func (r *CheckpointReader) loadEntries() error { r.entries = unique // Sort by end timestamp (newest first) - sort.Slice(r.entries, func(i, j int) bool { - ei, ej := r.entries[i].GetEnd(), r.entries[j].GetEnd() - return ei.GT(&ej) + slices.SortFunc(r.entries, func(a, b *checkpoint.CheckpointEntry) int { + ai, bi := a.GetEnd(), b.GetEnd() + return bi.Compare(&ai) }) return nil } @@ -258,8 +259,8 @@ func (r *CheckpointReader) GetAccounts(entry *checkpoint.CheckpointEntry) ([]*Ac for _, acc := range accountMap { accounts = append(accounts, acc) } - sort.Slice(accounts, func(i, j int) bool { - return accounts[i].AccountID < accounts[j].AccountID + slices.SortFunc(accounts, func(a, b *AccountInfo) int { + return cmp.Compare(a.AccountID, b.AccountID) }) return accounts, nil } @@ -311,8 +312,8 @@ func (r *CheckpointReader) rangesToTables(ranges []ckputil.TableRange) []*TableI for _, tbl := range tableMap { tables = append(tables, tbl) } - sort.Slice(tables, func(i, j int) bool { - return tables[i].TableID < tables[j].TableID + slices.SortFunc(tables, func(a, b *TableInfo) int { + return cmp.Compare(a.TableID, b.TableID) }) return tables } diff --git a/pkg/txn/client/operator.go b/pkg/txn/client/operator.go index 6de0a0828d88a..7445b2d134843 100644 --- a/pkg/txn/client/operator.go +++ b/pkg/txn/client/operator.go @@ -16,11 +16,12 @@ package client import ( "bytes" + "cmp" "context" "encoding/hex" "errors" "fmt" - "sort" + "slices" "strings" "sync" "sync/atomic" @@ -469,8 +470,8 @@ func (t *runSQLTracker) waitingInfo(keepToken uint64, skipToken uint64) []string infos = append(infos, waitInfo{token: token, sql: info.sql, start: info.start}) } } - sort.Slice(infos, func(i, j int) bool { - return infos[i].token < infos[j].token + slices.SortFunc(infos, func(a, b waitInfo) int { + return cmp.Compare(a.token, b.token) }) sqls := make([]string, 0, len(infos)) for _, info := range infos { diff --git a/pkg/txn/storage/memorystorage/mem_handler.go b/pkg/txn/storage/memorystorage/mem_handler.go index 2402b599d1c53..6bfea97cb0d09 100644 --- a/pkg/txn/storage/memorystorage/mem_handler.go +++ b/pkg/txn/storage/memorystorage/mem_handler.go @@ -15,13 +15,14 @@ package memorystorage import ( + "cmp" "context" crand "crypto/rand" "database/sql" "encoding/binary" "errors" "fmt" - "sort" + "slices" "sync" "github.com/matrixorigin/matrixone/pkg/catalog" @@ -842,8 +843,8 @@ func (m *MemHandler) HandleGetTableColumns(ctx context.Context, meta txn.TxnMeta ); err != nil { return err } - sort.Slice(attrRows, func(i, j int) bool { - return attrRows[i].Order < attrRows[j].Order + slices.SortFunc(attrRows, func(a, b *AttributeRow) int { + return cmp.Compare(a.Order, b.Order) }) for _, row := range attrRows { resp.Attrs = append(resp.Attrs, row.Attribute) @@ -908,8 +909,8 @@ func (m *MemHandler) HandleGetTableDefs(ctx context.Context, meta txn.TxnMeta, r return err } - sort.Slice(attrRows, func(i, j int) bool { - return attrRows[i].Order < attrRows[j].Order + slices.SortFunc(attrRows, func(a, b *AttributeRow) int { + return cmp.Compare(a.Order, b.Order) }) for _, row := range attrRows { def := &engine.AttributeDef{ diff --git a/pkg/util/debug/goroutine/analyze.go b/pkg/util/debug/goroutine/analyze.go index 5de19d165aef8..52b282a26e7e1 100644 --- a/pkg/util/debug/goroutine/analyze.go +++ b/pkg/util/debug/goroutine/analyze.go @@ -16,7 +16,8 @@ package goroutine import ( "bytes" - "sort" + "cmp" + "slices" "github.com/matrixorigin/matrixone/pkg/util/profile" ) @@ -113,8 +114,8 @@ func (z *analyzer) group( handle(i) } } - sort.Slice(groups, func(i, j int) bool { - return len(groups[i]) > len(groups[j]) + slices.SortFunc(groups, func(a, b []int) int { + return cmp.Compare(len(b), len(a)) }) return groups } diff --git a/pkg/vectorindex/cuvs/cdc.go b/pkg/vectorindex/cuvs/cdc.go index 30533f0789adb..647e25e00f04b 100644 --- a/pkg/vectorindex/cuvs/cdc.go +++ b/pkg/vectorindex/cuvs/cdc.go @@ -21,13 +21,14 @@ package cuvs import ( + "cmp" "encoding/binary" "encoding/hex" "encoding/json" "fmt" "hash/crc32" "math" - "sort" + "slices" "strings" "github.com/bytedance/sonic" @@ -435,7 +436,7 @@ type EventChunk struct { // SortChunks sorts chunks ascending by chunk_id in place. func SortChunks(chunks []EventChunk) { - sort.Slice(chunks, func(i, j int) bool { return chunks[i].ChunkId < chunks[j].ChunkId }) + slices.SortFunc(chunks, func(a, b EventChunk) int { return cmp.Compare(a.ChunkId, b.ChunkId) }) } // ReplayState is the post-replay output: pkids deleted in the main index and @@ -579,8 +580,8 @@ func ReplayEventLog( } // Stable order so callers (and tests) get deterministic output regardless // of map iteration order. - sort.Slice(out.Deleted, func(i, j int) bool { return out.Deleted[i] < out.Deleted[j] }) - sort.Slice(out.Overflow, func(i, j int) bool { return out.Overflow[i].Pkid < out.Overflow[j].Pkid }) + slices.Sort(out.Deleted) + slices.SortFunc(out.Overflow, func(a, b OverflowEntry) int { return cmp.Compare(a.Pkid, b.Pkid) }) return out, nil } diff --git a/pkg/vm/engine/disttae/cache/catalog.go b/pkg/vm/engine/disttae/cache/catalog.go index 8a67c85d0ca98..9adcea2fd0fb2 100644 --- a/pkg/vm/engine/disttae/cache/catalog.go +++ b/pkg/vm/engine/disttae/cache/catalog.go @@ -15,7 +15,8 @@ package cache import ( - "sort" + "cmp" + "slices" "strings" "sync" @@ -582,7 +583,7 @@ func ParseColumnsBatchAnd(bat *batch.Batch, f func(map[TableItemKey]Columns)) { } func InitTableItemWithColumns(item *TableItem, cols Columns) { - sort.Sort(cols) + slices.SortFunc(cols, func(a, b catalog.Column) int { return cmp.Compare(a.Num, b.Num) }) coldefs := make([]engine.TableDef, 0, len(cols)) for i, col := range cols { if col.ConstraintType == catalog.SystemColPKConstraint { diff --git a/pkg/vm/engine/disttae/local_disttae_datasource.go b/pkg/vm/engine/disttae/local_disttae_datasource.go index 1a8ee2aed6695..96e301f283e60 100644 --- a/pkg/vm/engine/disttae/local_disttae_datasource.go +++ b/pkg/vm/engine/disttae/local_disttae_datasource.go @@ -19,7 +19,6 @@ import ( "context" "fmt" "slices" - "sort" "strings" "github.com/matrixorigin/matrixone/pkg/catalog" @@ -338,28 +337,46 @@ func (ls *LocalDisttaeDataSource) sortBlockList() { ls.rangeSlice = make(objectio.BlockInfoSlice, ls.rangeSlice.Size()) if ls.desc { - sort.Slice(helper, func(i, j int) bool { - zm1 := helper[i].zm + less := func(a, b *blockSortHelper) bool { + zm1 := a.zm if !zm1.IsInited() { return true } - zm2 := helper[j].zm + zm2 := b.zm if !zm2.IsInited() { return false } return zm1.CompareMax(zm2) > 0 + } + slices.SortFunc(helper, func(a, b *blockSortHelper) int { + if less(a, b) { + return -1 + } + if less(b, a) { + return 1 + } + return 0 }) } else { - sort.Slice(helper, func(i, j int) bool { - zm1 := helper[i].zm + less := func(a, b *blockSortHelper) bool { + zm1 := a.zm if !zm1.IsInited() { return true } - zm2 := helper[j].zm + zm2 := b.zm if !zm2.IsInited() { return false } return zm1.CompareMin(zm2) < 0 + } + slices.SortFunc(helper, func(a, b *blockSortHelper) int { + if less(a, b) { + return -1 + } + if less(b, a) { + return 1 + } + return 0 }) } diff --git a/pkg/vm/engine/disttae/logtailreplay/get_flush_ts.go b/pkg/vm/engine/disttae/logtailreplay/get_flush_ts.go index 3eed74a664ad6..1d377515e0905 100644 --- a/pkg/vm/engine/disttae/logtailreplay/get_flush_ts.go +++ b/pkg/vm/engine/disttae/logtailreplay/get_flush_ts.go @@ -15,7 +15,7 @@ package logtailreplay import ( - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/container/types" ) @@ -26,8 +26,8 @@ func (p *PartitionState) GetFlushTS() types.TS { } rows := p.rows.Items() - sort.Slice(rows, func(i, j int) bool { - return rows[i].Time.Compare(&rows[j].Time) < 0 + slices.SortFunc(rows, func(a, b *RowEntry) int { + return a.Time.Compare(&b.Time) }) return rows[0].Time.Prev() } diff --git a/pkg/vm/engine/disttae/tracked_partitions.go b/pkg/vm/engine/disttae/tracked_partitions.go index 514a61214cd6a..77343be947517 100644 --- a/pkg/vm/engine/disttae/tracked_partitions.go +++ b/pkg/vm/engine/disttae/tracked_partitions.go @@ -15,7 +15,7 @@ package disttae import ( - "sort" + "slices" "sync" "sync/atomic" "time" @@ -160,9 +160,8 @@ func (tp *TrackedPartitions) Add( // Only evict if maxCount > 0 and we have reached the limit if maxCount > 0 && len(tp.snaps) >= maxCount { // Sort by last access time (oldest first) - sort.Slice(tp.snaps, func(i, j int) bool { - return tp.snaps[i].GetLastAccessTime().Before( - tp.snaps[j].GetLastAccessTime()) + slices.SortFunc(tp.snaps, func(a, b *TrackedPartition) int { + return a.GetLastAccessTime().Compare(b.GetLastAccessTime()) }) // Evict oldest snapshots to make room for new one diff --git a/pkg/vm/engine/disttae/txn.go b/pkg/vm/engine/disttae/txn.go index 3099bb78fa717..ad46150226bf1 100644 --- a/pkg/vm/engine/disttae/txn.go +++ b/pkg/vm/engine/disttae/txn.go @@ -15,12 +15,13 @@ package disttae import ( + "cmp" "context" "encoding/hex" "fmt" "math" "runtime" - "sort" + "slices" "strings" "sync" "time" @@ -792,8 +793,8 @@ func (txn *Transaction) dumpInsertBatchLocked( for k := range tbSize { keys = append(keys, k) } - sort.Slice(keys, func(i, j int) bool { - return tbSize[keys[i]] < tbSize[keys[j]] + slices.SortFunc(keys, func(a, b uint64) int { + return cmp.Compare(tbSize[a], tbSize[b]) }) // Skip the skipTable logic if force flush is enabled @@ -1536,9 +1537,7 @@ func (txn *Transaction) mergeTxnWorkspaceLocked(ctx context.Context) error { for _, e := range txn.writes { if sels, ok := txn.batchSelectList[e.bat]; ok { txn.approximateInMemInsertCnt -= len(sels) - sort.Slice(sels, func(i, j int) bool { - return sels[i] < (sels[j]) - }) + slices.Sort(sels) shrinkBatchWithRowids(e.bat, sels) delete(txn.batchSelectList, e.bat) } diff --git a/pkg/vm/engine/memoryengine/shard.go b/pkg/vm/engine/memoryengine/shard.go index 2ac52c2a11302..1bc8955b95276 100644 --- a/pkg/vm/engine/memoryengine/shard.go +++ b/pkg/vm/engine/memoryengine/shard.go @@ -15,8 +15,9 @@ package memoryengine import ( + "cmp" "context" - "sort" + "slices" "sync" "github.com/matrixorigin/matrixone/pkg/common/mpool" @@ -142,8 +143,8 @@ func (s *NoShard) setShard(stores []metadata.TNService) { if len(infos) == 0 { panic("no shard") } - sort.Slice(infos, func(i, j int) bool { - return infos[i].Shard.ShardID < infos[j].Shard.ShardID + slices.SortFunc(infos, func(a, b ShardInfo) int { + return cmp.Compare(a.Shard.ShardID, b.Shard.ShardID) }) info := infos[0] s.shard = Shard{ diff --git a/pkg/vm/engine/memoryengine/shard_hash.go b/pkg/vm/engine/memoryengine/shard_hash.go index 32301eb2910a5..4b9e40cddab3e 100644 --- a/pkg/vm/engine/memoryengine/shard_hash.go +++ b/pkg/vm/engine/memoryengine/shard_hash.go @@ -15,10 +15,11 @@ package memoryengine import ( + "cmp" "context" "fmt" "hash/fnv" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/mpool" @@ -67,8 +68,8 @@ func (*HashShard) Batch( primaryAttrs = append(primaryAttrs, attr.Attr) } } - sort.Slice(primaryAttrs, func(i, j int) bool { - return primaryAttrs[i].Name < primaryAttrs[j].Name + slices.SortFunc(primaryAttrs, func(a, b engine.Attribute) int { + return cmp.Compare(a.Name, b.Name) }) if len(primaryAttrs) == 0 { // no shard key @@ -103,8 +104,8 @@ func (*HashShard) Batch( }) } } - sort.Slice(shards, func(i, j int) bool { - return shards[i].ShardID < shards[j].ShardID + slices.SortFunc(shards, func(a, b *Shard) int { + return cmp.Compare(a.ShardID, b.ShardID) }) type batValue struct { @@ -202,8 +203,8 @@ func (h *HashShard) Vector( }) } } - sort.Slice(shards, func(i, j int) bool { - return shards[i].ShardID < shards[j].ShardID + slices.SortFunc(shards, func(a, b *Shard) int { + return cmp.Compare(a.ShardID, b.ShardID) }) m := make(map[*Shard]*vector.Vector) diff --git a/pkg/vm/engine/readutil/pk_filter.go b/pkg/vm/engine/readutil/pk_filter.go index fabcf11c60924..8326315322e22 100644 --- a/pkg/vm/engine/readutil/pk_filter.go +++ b/pkg/vm/engine/readutil/pk_filter.go @@ -16,7 +16,7 @@ package readutil import ( "bytes" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/mpool" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -290,7 +290,7 @@ func combineOffsetFuncs(funcs []func(*vector.Vector) []int64) func(*vector.Vecto out = append(out, off) } } - sort.Slice(out, func(i, j int) bool { return out[i] < out[j] }) + slices.Sort(out) return out } } diff --git a/pkg/vm/engine/readutil/tombstone_data.go b/pkg/vm/engine/readutil/tombstone_data.go index ebf04b6ad20a1..e74cb8fa4a2d8 100644 --- a/pkg/vm/engine/readutil/tombstone_data.go +++ b/pkg/vm/engine/readutil/tombstone_data.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "fmt" + "slices" "sort" "github.com/matrixorigin/matrixone/pkg/common/moerr" @@ -292,8 +293,8 @@ func (tomb *tombstoneData) ApplyPersistedTombstones( } func (tomb *tombstoneData) SortInMemory() { - sort.Slice(tomb.rowids, func(i, j int) bool { - return tomb.rowids[i].LT(&tomb.rowids[j]) + slices.SortFunc(tomb.rowids, func(a, b types.Rowid) int { + return a.Compare(&b) }) } diff --git a/pkg/vm/engine/tae/catalog/schema.go b/pkg/vm/engine/tae/catalog/schema.go index 7075859f92cb5..3b2ac6a0c3aa0 100644 --- a/pkg/vm/engine/tae/catalog/schema.go +++ b/pkg/vm/engine/tae/catalog/schema.go @@ -16,11 +16,11 @@ package catalog import ( "bytes" + "cmp" "encoding/json" "fmt" "io" "slices" - "sort" "strings" "time" @@ -118,7 +118,7 @@ func (cpk *SortKey) AddDef(def *ColDef) (ok bool) { cpk.isPrimary = true } cpk.Defs = append(cpk.Defs, def) - sort.Slice(cpk.Defs, func(i, j int) bool { return cpk.Defs[i].SortIdx < cpk.Defs[j].SortIdx }) + slices.SortFunc(cpk.Defs, func(a, b *ColDef) int { return cmp.Compare(a.SortIdx, b.SortIdx) }) cpk.search[def.Idx] = int(def.SortIdx) return true } diff --git a/pkg/vm/engine/tae/common/intervals.go b/pkg/vm/engine/tae/common/intervals.go index ce085a492a1f1..2472d48d808cd 100644 --- a/pkg/vm/engine/tae/common/intervals.go +++ b/pkg/vm/engine/tae/common/intervals.go @@ -15,8 +15,9 @@ package common import ( + "cmp" "io" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/container/types" "github.com/matrixorigin/matrixone/pkg/logutil" @@ -44,8 +45,8 @@ func (intervals *ClosedIntervals) GetMin() uint64 { } func (intervals *ClosedIntervals) TryMerge(o *ClosedIntervals) { intervals.Intervals = append(intervals.Intervals, o.Intervals...) - sort.Slice(intervals.Intervals, func(i, j int) bool { - return intervals.Intervals[i].Start < intervals.Intervals[j].Start + slices.SortFunc(intervals.Intervals, func(a, b *ClosedInterval) int { + return cmp.Compare(a.Start, b.Start) }) newIntervals := make([]*ClosedInterval, 0) if len(intervals.Intervals) == 0 { @@ -222,9 +223,7 @@ func NewClosedIntervalsBySlice(array []uint64) *ClosedIntervals { if len(array) == 0 { return ranges } - sort.Slice(array, func(i, j int) bool { - return array[i] < array[j] - }) + slices.Sort(array) interval := &ClosedInterval{Start: array[0]} pre := array[0] array = array[1:] diff --git a/pkg/vm/engine/tae/compute/compute.go b/pkg/vm/engine/tae/compute/compute.go index b23b3d0ab2bcd..b6b709c456dcb 100644 --- a/pkg/vm/engine/tae/compute/compute.go +++ b/pkg/vm/engine/tae/compute/compute.go @@ -16,7 +16,7 @@ package compute import ( "bytes" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/container/nulls" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -32,8 +32,14 @@ func SortAndDedup[T any]( if len(vals) < 1 { return vals } - sort.Slice(vals, func(i, j int) bool { - return lessFn(&vals[i], &vals[j]) + slices.SortFunc(vals, func(a, b T) int { + if lessFn(&a, &b) { + return -1 + } + if lessFn(&b, &a) { + return 1 + } + return 0 }) pos := 1 for start, end := 1, len(vals); start < end; start++ { diff --git a/pkg/vm/engine/tae/db/checkpoint/replay.go b/pkg/vm/engine/tae/db/checkpoint/replay.go index 1a70960ceedd5..b527e264608d0 100644 --- a/pkg/vm/engine/tae/db/checkpoint/replay.go +++ b/pkg/vm/engine/tae/db/checkpoint/replay.go @@ -17,7 +17,7 @@ package checkpoint import ( "context" "fmt" - "sort" + "slices" "sync" "time" @@ -116,8 +116,8 @@ func (c *CkpReplayer) readCheckpointEntries() ( return } - sort.Slice(files, func(i, j int) bool { - return files[i].GetEnd().LT(files[j].GetEnd()) + slices.SortFunc(files, func(a, b ioutil.TSRangeFile) int { + return a.GetEnd().Compare(b.GetEnd()) }) var ( @@ -501,8 +501,8 @@ func MergeCkpMeta( return "", nil } - sort.Slice(metaFiles, func(i, j int) bool { - return metaFiles[i].GetEnd().LT(metaFiles[j].GetEnd()) + slices.SortFunc(metaFiles, func(a, b ioutil.TSRangeFile) int { + return a.GetEnd().Compare(b.GetEnd()) }) maxFile := metaFiles[len(metaFiles)-1] diff --git a/pkg/vm/engine/tae/db/checkpoint/snapshot.go b/pkg/vm/engine/tae/db/checkpoint/snapshot.go index 405a73e5f9bd4..3ed58793f609a 100644 --- a/pkg/vm/engine/tae/db/checkpoint/snapshot.go +++ b/pkg/vm/engine/tae/db/checkpoint/snapshot.go @@ -16,7 +16,7 @@ package checkpoint import ( "context" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/objectio/ioutil" @@ -83,12 +83,12 @@ func getSnapshotMetaFiles( metaFiles, compactedFiles []ioutil.TSRangeFile, snapshot *types.TS, ) []ioutil.TSRangeFile { - sort.Slice(compactedFiles, func(i, j int) bool { - return compactedFiles[i].GetEnd().LT(compactedFiles[j].GetEnd()) + slices.SortFunc(compactedFiles, func(a, b ioutil.TSRangeFile) int { + return a.GetEnd().Compare(b.GetEnd()) }) - sort.Slice(metaFiles, func(i, j int) bool { - return metaFiles[i].GetEnd().LT(metaFiles[j].GetEnd()) + slices.SortFunc(metaFiles, func(a, b ioutil.TSRangeFile) int { + return a.GetEnd().Compare(b.GetEnd()) }) retFiles := make([]ioutil.TSRangeFile, 0) @@ -223,11 +223,11 @@ func filterSnapshotEntries(entries []*CheckpointEntry, snapshot *types.TS) []*Ch } // Sort by end timestamp - sort.Slice(entries, func(i, j int) bool { - if entries[i] == nil || entries[j] == nil { - return false + slices.SortFunc(entries, func(a, b *CheckpointEntry) int { + if a == nil || b == nil { + return 0 } - return entries[i].end.LT(&entries[j].end) + return a.end.Compare(&b.end) }) if snapshot != nil && snapshot.Equal(&maxGlobalEnd) { diff --git a/pkg/vm/engine/tae/db/merge/statLayerZero.go b/pkg/vm/engine/tae/db/merge/statLayerZero.go index d22928692c6a6..297540c4f72ec 100644 --- a/pkg/vm/engine/tae/db/merge/statLayerZero.go +++ b/pkg/vm/engine/tae/db/merge/statLayerZero.go @@ -20,7 +20,7 @@ import ( "math" "math/bits" "math/rand" - "sort" + "slices" "time" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -285,14 +285,14 @@ func GatherLayerZeroMergeTasks(ctx context.Context, typ := objList[0].SortKeyZoneMap().GetType() scale := objList[0].SortKeyZoneMap().GetScale() if typ != types.T_any { - sort.Slice(objList, func(i, j int) bool { + slices.SortFunc(objList, func(a, b *objectio.ObjectStats) int { return compute.Compare( - objList[i].SortKeyZoneMap().GetMinBuf(), - objList[j].SortKeyZoneMap().GetMinBuf(), + a.SortKeyZoneMap().GetMinBuf(), + b.SortKeyZoneMap().GetMinBuf(), typ, scale, scale, - ) < 0 + ) }) } for _, obj := range objList { diff --git a/pkg/vm/engine/tae/db/merge/statOverlap.go b/pkg/vm/engine/tae/db/merge/statOverlap.go index fc87eb27c62ff..6f1e79eabb922 100644 --- a/pkg/vm/engine/tae/db/merge/statOverlap.go +++ b/pkg/vm/engine/tae/db/merge/statOverlap.go @@ -15,9 +15,10 @@ package merge import ( + "cmp" "context" "fmt" - "sort" + "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -423,11 +424,11 @@ func splitTasksOnSpan( records := overlapStats.StatsRecords // sort by overlap ratio descending - sort.Slice(candidatesIdx, func(i, j int) bool { - r1, r2 := records[candidatesIdx[i]], records[candidatesIdx[j]] + slices.SortFunc(candidatesIdx, func(a, b int) int { + r1, r2 := records[a], records[b] overlapRatio1 := float64(r1.overlap) / float64(r1.depth) overlapRatio2 := float64(r2.overlap) / float64(r2.depth) - return overlapRatio1 > overlapRatio2 + return cmp.Compare(overlapRatio2, overlapRatio1) }) pushToRationLessThan := func(idx int, ration float64) int { diff --git a/pkg/vm/engine/tae/logstore/driver/batchstoredriver/file.go b/pkg/vm/engine/tae/logstore/driver/batchstoredriver/file.go index f6142ef921dcf..fb92055980ba2 100644 --- a/pkg/vm/engine/tae/logstore/driver/batchstoredriver/file.go +++ b/pkg/vm/engine/tae/logstore/driver/batchstoredriver/file.go @@ -14,12 +14,13 @@ package batchstoredriver import ( + "cmp" "context" "fmt" "os" "path" "path/filepath" - "sort" + "slices" "strconv" "strings" "sync" @@ -138,8 +139,8 @@ func OpenRotateFile(dir, name string, mu *sync.RWMutex, rotateChecker RotateChec return nil, err } } else { - sort.Slice(vfiles, func(i, j int) bool { - return vfiles[i].(*vFile).version < vfiles[j].(*vFile).version + slices.SortFunc(vfiles, func(a, b VFile) int { + return cmp.Compare(a.(*vFile).version, b.(*vFile).version) }) observer.onTruncatedFile(vfiles[0].Id() - 1) rf.history.Extend(vfiles[:len(vfiles)-1]...) diff --git a/pkg/vm/engine/tae/logtail/ckp_reader.go b/pkg/vm/engine/tae/logtail/ckp_reader.go index 3704a15f4d279..46da162d36e5a 100644 --- a/pkg/vm/engine/tae/logtail/ckp_reader.go +++ b/pkg/vm/engine/tae/logtail/ckp_reader.go @@ -15,9 +15,10 @@ package logtail import ( + "cmp" "context" "fmt" - "sort" + "slices" "strconv" "strings" "time" @@ -860,8 +861,8 @@ func getMetaInfo( objectCount += count.delete deleteCount += count.delete } - sort.Slice(tableinfos, func(i, j int) bool { - return tableinfos[i].add > tableinfos[j].add + slices.SortFunc(tableinfos, func(a, b *tableinfo) int { + return cmp.Compare(b.add, a.add) }) tableJsons := make([]TableInfoJson, 0, objBatchLength) tables := make(map[uint64]int) @@ -884,8 +885,8 @@ func getMetaInfo( objectCount2 += count.add addCount2 += count.add } - sort.Slice(tableinfos2, func(i, j int) bool { - return tableinfos2[i].add > tableinfos2[j].add + slices.SortFunc(tableinfos2, func(a, b *tableinfo) int { + return cmp.Compare(b.add, a.add) }) for i := range len(tableinfos2) { diff --git a/pkg/vm/engine/tae/logtail/handle.go b/pkg/vm/engine/tae/logtail/handle.go index c0158b6957909..6192caab094aa 100644 --- a/pkg/vm/engine/tae/logtail/handle.go +++ b/pkg/vm/engine/tae/logtail/handle.go @@ -70,7 +70,7 @@ Main workflow: import ( "context" "fmt" - "sort" + "slices" "time" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -350,7 +350,7 @@ func (b *TableLogtailRespBuilder) BuildResp() (api.SyncLogTailResp, error) { for k := range b.dataInsBatches { keys = append(keys, k) } - sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) + slices.Sort(keys) for _, k := range keys { if err := tryAppendEntry(api.Entry_Insert, TableRespKind_Data, DataChangeToLogtailBatch(b.dataInsBatches[k]), k); err != nil { return empty, err diff --git a/pkg/vm/engine/tae/logtail/snapshot.go b/pkg/vm/engine/tae/logtail/snapshot.go index 873a78fcfd8a2..351024fb2e042 100644 --- a/pkg/vm/engine/tae/logtail/snapshot.go +++ b/pkg/vm/engine/tae/logtail/snapshot.go @@ -18,7 +18,7 @@ import ( "bytes" "context" "fmt" - "sort" + "slices" "sync" "time" @@ -575,8 +575,8 @@ func (sm *SnapshotMeta) updateTableInfo( for _, info := range tObjects { orderedInfos = append(orderedInfos, info) } - sort.Slice(orderedInfos, func(i, j int) bool { - return orderedInfos[i].createAt.LT(&orderedInfos[j].createAt) + slices.SortFunc(orderedInfos, func(a, b *objectInfo) int { + return a.createAt.Compare(&b.createAt) }) for _, obj := range orderedInfos { @@ -728,9 +728,8 @@ func (sm *SnapshotMeta) updateTableInfo( }) } } - sort.Slice(deleteRows, func(i, j int) bool { - ts2 := deleteRows[j].ts - return deleteRows[i].ts.LT(&ts2) + slices.SortFunc(deleteRows, func(a, b tombstone) int { + return a.ts.Compare(&b.ts) }) for _, delRow := range deleteRows { @@ -1133,8 +1132,8 @@ func (sm *SnapshotMeta) GetSnapshot( } // Sort cluster snapshots - sort.Slice(snapshotInfo.cluster, func(i, j int) bool { - return snapshotInfo.cluster[i].LT(&snapshotInfo.cluster[j]) + slices.SortFunc(snapshotInfo.cluster, func(a, b types.TS) int { + return a.Compare(&b) }) logutil.Info( "GetSnapshot-P3-Cluster", @@ -1143,8 +1142,8 @@ func (sm *SnapshotMeta) GetSnapshot( // Sort account snapshots for accountID, tsList := range snapshotInfo.account { - sort.Slice(tsList, func(i, j int) bool { - return tsList[i].LT(&tsList[j]) + slices.SortFunc(tsList, func(a, b types.TS) int { + return a.Compare(&b) }) snapshotInfo.account[accountID] = tsList logutil.Info( @@ -1156,8 +1155,8 @@ func (sm *SnapshotMeta) GetSnapshot( // Sort database snapshots for dbID, tsList := range snapshotInfo.database { - sort.Slice(tsList, func(i, j int) bool { - return tsList[i].LT(&tsList[j]) + slices.SortFunc(tsList, func(a, b types.TS) int { + return a.Compare(&b) }) snapshotInfo.database[dbID] = tsList logutil.Info( @@ -1169,8 +1168,8 @@ func (sm *SnapshotMeta) GetSnapshot( // Sort table snapshots for tableID, tsList := range snapshotInfo.tables { - sort.Slice(tsList, func(i, j int) bool { - return tsList[i].LT(&tsList[j]) + slices.SortFunc(tsList, func(a, b types.TS) int { + return a.Compare(&b) }) snapshotInfo.tables[tableID] = tsList logutil.Info( diff --git a/pkg/vm/engine/tae/txn/txnimpl/table.go b/pkg/vm/engine/tae/txn/txnimpl/table.go index c8642ed032085..5da946e4e0fdf 100644 --- a/pkg/vm/engine/tae/txn/txnimpl/table.go +++ b/pkg/vm/engine/tae/txn/txnimpl/table.go @@ -19,7 +19,7 @@ import ( "context" "fmt" "runtime/trace" - "sort" + "slices" "strings" "sync/atomic" "time" @@ -269,8 +269,15 @@ func (tbl *txnTable) TransferDeletes( } } softDeleteObjects = tbl.entry.GetSoftdeleteObjects(startTS, ts) - sort.Slice(softDeleteObjects, func(i, j int) bool { - return softDeleteObjects[i].CreatedAt.LE(&softDeleteObjects[j].CreatedAt) + // Preserve the exact ordering of the original sort.Slice, whose comparator + // was non-strict (CreatedAt.LE); a strict 3-way Compare (0 on ties) would + // reorder objects created at the same timestamp. Keep the original order + // while avoiding sort.Slice's interface{} boxing. + slices.SortFunc(softDeleteObjects, func(a, b *catalog.ObjectEntry) int { + if a.CreatedAt.LE(&b.CreatedAt) { + return -1 + } + return 1 }) v2.TxnS3TombstoneTransferGetSoftdeleteObjectsHistogram.Observe(time.Since(tGetSoftdeleteObjects).Seconds()) v2.TxnS3TombstoneSoftdeleteObjectCounter.Add(float64(len(softDeleteObjects))) From d6dd66eddb5cd27f7a681a82bfc5e5ac5c79ba32 Mon Sep 17 00:00:00 2001 From: fengttt Date: Sun, 12 Jul 2026 10:38:16 -0700 Subject: [PATCH 2/6] fix(review): use strict comparators; keep semantic filter sort on sort.Slice Address review: a slices.SortFunc comparator must be a strict weak ordering. Four sites were not: - pkg/vm/engine/disttae/local_disttae_datasource.go: the block-order comparator returned "less" in both directions when both zone maps were uninitialized. Compare initialization state explicitly first (both uninitialized => equal, exactly one => a consistent sign), then compare min/max. Valid in both ASC and DESC branches. - pkg/vm/engine/tae/db/checkpoint/snapshot.go: nil entries were treated as equal to every entry (non-transitive incomparability). Give nil a deterministic position (sorted first, both-nil => 0); dereferences stay nil-safe. - pkg/vm/engine/tae/txn/txnimpl/table.go: the soft-deleted-object sort used a non-strict CreatedAt.LE. Use slices.SortStableFunc with a strict CreatedAt.Compare so equal timestamps keep their original relative order. - pkg/sql/plan/stats.go: the filter-cost sort's comparator is non-strict (cost1 <= cost2) and its exact tie ordering is load-bearing -- it selects which predicate a vector/IVF index probes with and which value a runtime bool error reports (see operator/is_not_operator, vector/vector_ivf_mode, and IVF issue #25639). No strict slices comparator reproduces that order (a stable strict sort changes it and returns wrong results), so this one site is intentionally kept on sort.Slice. Verified: go build ./..., go vet, gofmt clean; BVT operator, vector, optimizer, dml/{select,delete,insert}, join, dtype, disttae all 100%. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/sql/plan/stats.go | 28 +++++---- .../disttae/local_disttae_datasource.go | 58 ++++++++----------- pkg/vm/engine/tae/db/checkpoint/snapshot.go | 12 +++- pkg/vm/engine/tae/txn/txnimpl/table.go | 14 ++--- 4 files changed, 52 insertions(+), 60 deletions(-) diff --git a/pkg/sql/plan/stats.go b/pkg/sql/plan/stats.go index 2281f5b27179f..70e6d982f874d 100644 --- a/pkg/sql/plan/stats.go +++ b/pkg/sql/plan/stats.go @@ -19,7 +19,7 @@ import ( "context" "fmt" "math" - "slices" + "sort" "strconv" "strings" "sync/atomic" @@ -1069,20 +1069,18 @@ func sortFilterListByStats(ctx context.Context, nodeID int32, builder *QueryBuil switch node.NodeType { case plan.Node_TABLE_SCAN: if node.ObjRef != nil && len(node.FilterList) >= 1 { - // Filter evaluation order here is semantically significant: it decides - // which predicate a vector/IVF index probes with and which value a - // runtime error reports (see operator/is_not_operator, - // vector/vector_ivf_mode). The original sort.Slice comparator was - // non-strict (cost1 <= cost2); a strict cmp.Compare (0 on ties) would - // reorder equal-cost filters and change results. Reproduce the exact - // original ordering here while avoiding sort.Slice's interface{} boxing. - slices.SortFunc(node.FilterList, func(a, b *plan.Expr) int { - cost1 := estimateFilterWeight(a, 0) * a.Selectivity - cost2 := estimateFilterWeight(b, 0) * b.Selectivity - if cost1 <= cost2 { - return -1 - } - return 1 + // NB: intentionally NOT migrated to slices.SortFunc. Filter evaluation + // order is semantically significant here (it decides which predicate a + // vector/IVF index probes with and which value a runtime error reports + // -- see operator/is_not_operator and vector/vector_ivf_mode, and the + // underlying IVF issue #25639). The comparator is non-strict (returns + // true on equal cost) and its exact tie ordering is an artifact of + // sort.Slice that no strict slices comparator reproduces, so this site + // is left on sort.Slice. + sort.Slice(node.FilterList, func(i, j int) bool { + cost1 := estimateFilterWeight(node.FilterList[i], 0) * node.FilterList[i].Selectivity + cost2 := estimateFilterWeight(node.FilterList[j], 0) * node.FilterList[j].Selectivity + return cost1 <= cost2 }) } } diff --git a/pkg/vm/engine/disttae/local_disttae_datasource.go b/pkg/vm/engine/disttae/local_disttae_datasource.go index 96e301f283e60..b907ca1d6e8ea 100644 --- a/pkg/vm/engine/disttae/local_disttae_datasource.go +++ b/pkg/vm/engine/disttae/local_disttae_datasource.go @@ -336,47 +336,37 @@ func (ls *LocalDisttaeDataSource) sortBlockList() { } ls.rangeSlice = make(objectio.BlockInfoSlice, ls.rangeSlice.Size()) - if ls.desc { - less := func(a, b *blockSortHelper) bool { - zm1 := a.zm - if !zm1.IsInited() { - return true - } - zm2 := b.zm - if !zm2.IsInited() { - return false - } - return zm1.CompareMax(zm2) > 0 + // compareInit orders uninitialized zone maps before initialized ones and + // treats two uninitialized zone maps as equal, so the comparator is a valid + // strict weak ordering (unlike a bare min/max compare, which is undefined for + // uninitialized zone maps). It returns (result, done): done is false only when + // both zone maps are initialized and the caller must compare values. + compareInit := func(a, b *blockSortHelper) (int, bool) { + ai, bi := a.zm.IsInited(), b.zm.IsInited() + if ai && bi { + return 0, false } + if ai == bi { + return 0, true // both uninitialized: equal + } + if !ai { + return -1, true // uninitialized sorts first + } + return 1, true + } + if ls.desc { slices.SortFunc(helper, func(a, b *blockSortHelper) int { - if less(a, b) { - return -1 - } - if less(b, a) { - return 1 + if r, done := compareInit(a, b); done { + return r } - return 0 + return b.zm.CompareMax(a.zm) // descending by max }) } else { - less := func(a, b *blockSortHelper) bool { - zm1 := a.zm - if !zm1.IsInited() { - return true - } - zm2 := b.zm - if !zm2.IsInited() { - return false - } - return zm1.CompareMin(zm2) < 0 - } slices.SortFunc(helper, func(a, b *blockSortHelper) int { - if less(a, b) { - return -1 - } - if less(b, a) { - return 1 + if r, done := compareInit(a, b); done { + return r } - return 0 + return a.zm.CompareMin(b.zm) // ascending by min }) } diff --git a/pkg/vm/engine/tae/db/checkpoint/snapshot.go b/pkg/vm/engine/tae/db/checkpoint/snapshot.go index 3ed58793f609a..6787928656c38 100644 --- a/pkg/vm/engine/tae/db/checkpoint/snapshot.go +++ b/pkg/vm/engine/tae/db/checkpoint/snapshot.go @@ -222,10 +222,18 @@ func filterSnapshotEntries(entries []*CheckpointEntry, snapshot *types.TS) []*Ch } } - // Sort by end timestamp + // Sort by end timestamp. nil entries are given a deterministic position + // (sorted first) so the comparator is a valid strict weak ordering; treating + // nil as equal to every entry would make incomparability non-transitive. slices.SortFunc(entries, func(a, b *CheckpointEntry) int { if a == nil || b == nil { - return 0 + if a == b { + return 0 + } + if a == nil { + return -1 + } + return 1 } return a.end.Compare(&b.end) }) diff --git a/pkg/vm/engine/tae/txn/txnimpl/table.go b/pkg/vm/engine/tae/txn/txnimpl/table.go index 5da946e4e0fdf..61f779eccf2c8 100644 --- a/pkg/vm/engine/tae/txn/txnimpl/table.go +++ b/pkg/vm/engine/tae/txn/txnimpl/table.go @@ -269,15 +269,11 @@ func (tbl *txnTable) TransferDeletes( } } softDeleteObjects = tbl.entry.GetSoftdeleteObjects(startTS, ts) - // Preserve the exact ordering of the original sort.Slice, whose comparator - // was non-strict (CreatedAt.LE); a strict 3-way Compare (0 on ties) would - // reorder objects created at the same timestamp. Keep the original order - // while avoiding sort.Slice's interface{} boxing. - slices.SortFunc(softDeleteObjects, func(a, b *catalog.ObjectEntry) int { - if a.CreatedAt.LE(&b.CreatedAt) { - return -1 - } - return 1 + // Stable sort with a strict comparator: objects created at the same + // timestamp keep their original relative order (the tie-breaker), instead + // of sort.Slice's unspecified tie ordering. + slices.SortStableFunc(softDeleteObjects, func(a, b *catalog.ObjectEntry) int { + return a.CreatedAt.Compare(&b.CreatedAt) }) v2.TxnS3TombstoneTransferGetSoftdeleteObjectsHistogram.Observe(time.Since(tGetSoftdeleteObjects).Seconds()) v2.TxnS3TombstoneSoftdeleteObjectCounter.Add(float64(len(softDeleteObjects))) From edd3c9fba49daaccb8ea508317269489f56af874 Mon Sep 17 00:00:00 2001 From: fengttt Date: Sun, 12 Jul 2026 11:25:54 -0700 Subject: [PATCH 3/6] fix(vector): fold multiple distance predicates into the tightest range; use stable filter sort Fixes #25639: an IVFFLAT `mode=pre` query with more than one distance predicate on the indexed column (e.g. `l2 < 1.1 AND l2 < 1.2`) could return wrong rows. getDistRangeFromFilters kept only the FIRST bound per side as the index range and left the rest as residual filters, which the mode=pre join then dropped -- so the result depended on filter order (7 rows instead of 3 when the looser bound came first). Now getDistRangeFromFilters folds every same-side bound into the tightest one (min for upper, max for lower; exclusive wins on an equal value), so the index enforces the intersection of all predicates regardless of order. A looser same-side bound is redundant and dropped; a non-literal bound still falls back to being kept as a residual filter. Verified: `l2 < 1.1 AND l2 < 1.2` and `l2 < 1.2 AND l2 < 1.1` both return the correct 3 rows. Fixes #25646: with the IVF scan no longer order-sensitive, the filter-cost sort in stats.go can be a well-defined ordering again. Switch it from sort.Slice to slices.SortStableFunc with a strict cmp.Compare: equal-cost filters keep their original (as-written) relative order -- a deterministic, valid strict weak ordering -- instead of sort.Slice's unspecified tie order. This completes the sort.Slice -> slices migration for this site. Golden updates (behavior is otherwise unchanged): - optimizer/in_domain, optimizer/associative: EXPLAIN now renders equal-cost filters in as-written order (display only). - operator/is_not_operator: `str2 IS TRUE AND str1 IS TRUE` (an invalid query) now cites str2's invalid value first; still an error either way. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/sql/plan/apply_indices_vector.go | 98 ++++++++++++++++--- pkg/sql/plan/apply_indices_vector_test.go | 41 ++++++-- pkg/sql/plan/stats.go | 27 ++--- .../cases/operator/is_not_operator.result | 2 +- .../cases/optimizer/associative.result | 2 +- .../cases/optimizer/in_domain.result | 14 +-- .../cases/vector/vector_ivf_mode.sql | 5 +- 7 files changed, 143 insertions(+), 46 deletions(-) diff --git a/pkg/sql/plan/apply_indices_vector.go b/pkg/sql/plan/apply_indices_vector.go index a7d53714b53c9..eccf5766b3e51 100644 --- a/pkg/sql/plan/apply_indices_vector.go +++ b/pkg/sql/plan/apply_indices_vector.go @@ -718,46 +718,43 @@ func (builder *QueryBuilder) getDistRangeFromFilters( goto NO_RANGE } + // Fold every matching bound into the range, keeping the tightest bound per + // side so the index enforces the intersection of all predicates regardless + // of filter order (a looser same-side bound is redundant and dropped). If a + // bound is not a comparable literal, the predicate is kept as a residual + // filter instead. See issue #25639. switch f.Func.ObjName { case "<": if distRange == nil { distRange = &plan.DistRange{} } - if distRange.UpperBoundType != plan.BoundType_UNBOUNDED { + if !mergeUpperBound(distRange, f.Args[1], plan.BoundType_EXCLUSIVE) { goto NO_RANGE } - distRange.UpperBoundType = plan.BoundType_EXCLUSIVE - distRange.UpperBound = f.Args[1] case "<=": if distRange == nil { distRange = &plan.DistRange{} } - if distRange.UpperBoundType != plan.BoundType_UNBOUNDED { + if !mergeUpperBound(distRange, f.Args[1], plan.BoundType_INCLUSIVE) { goto NO_RANGE } - distRange.UpperBoundType = plan.BoundType_INCLUSIVE - distRange.UpperBound = f.Args[1] case ">": if distRange == nil { distRange = &plan.DistRange{} } - if distRange.LowerBoundType != plan.BoundType_UNBOUNDED { + if !mergeLowerBound(distRange, f.Args[1], plan.BoundType_EXCLUSIVE) { goto NO_RANGE } - distRange.LowerBoundType = plan.BoundType_EXCLUSIVE - distRange.LowerBound = f.Args[1] case ">=": if distRange == nil { distRange = &plan.DistRange{} } - if distRange.LowerBoundType != plan.BoundType_UNBOUNDED { + if !mergeLowerBound(distRange, f.Args[1], plan.BoundType_INCLUSIVE) { goto NO_RANGE } - distRange.LowerBoundType = plan.BoundType_INCLUSIVE - distRange.LowerBound = f.Args[1] default: goto NO_RANGE @@ -773,6 +770,83 @@ func (builder *QueryBuilder) getDistRangeFromFilters( return filters[:currIdx], distRange } +// distBoundLiteralFloat64 extracts a comparable float64 from a distance-bound +// literal expression. It accepts the same numeric literal kinds the index reader +// accepts (getLiteralFloat64), returning ok=false for anything that is not a +// plain numeric literal. +func distBoundLiteralFloat64(expr *plan.Expr) (float64, bool) { + lit := expr.GetLit() + if lit == nil || lit.Isnull { + return 0, false + } + switch v := lit.Value.(type) { + case *plan.Literal_Fval: + return float64(v.Fval), true + case *plan.Literal_Dval: + return v.Dval, true + case *plan.Literal_I8Val: + return float64(v.I8Val), true + case *plan.Literal_I16Val: + return float64(v.I16Val), true + case *plan.Literal_I32Val: + return float64(v.I32Val), true + case *plan.Literal_I64Val: + return float64(v.I64Val), true + case *plan.Literal_U8Val: + return float64(v.U8Val), true + case *plan.Literal_U16Val: + return float64(v.U16Val), true + case *plan.Literal_U32Val: + return float64(v.U32Val), true + case *plan.Literal_U64Val: + return float64(v.U64Val), true + default: + return 0, false + } +} + +// mergeUpperBound folds a new upper bound into dr, keeping the tighter (smaller, +// or exclusive on an equal value) bound so the range is the intersection of all +// upper bounds. It returns false when the bounds cannot be compared as numeric +// literals, in which case the caller keeps the predicate as a residual filter. +func mergeUpperBound(dr *plan.DistRange, bound *plan.Expr, boundType plan.BoundType) bool { + if dr.UpperBoundType == plan.BoundType_UNBOUNDED { + dr.UpperBoundType = boundType + dr.UpperBound = bound + return true + } + newVal, ok1 := distBoundLiteralFloat64(bound) + curVal, ok2 := distBoundLiteralFloat64(dr.UpperBound) + if !ok1 || !ok2 { + return false + } + if newVal < curVal || (newVal == curVal && boundType == plan.BoundType_EXCLUSIVE) { + dr.UpperBoundType = boundType + dr.UpperBound = bound + } + return true +} + +// mergeLowerBound folds a new lower bound into dr, keeping the tighter (larger, +// or exclusive on an equal value) bound. See mergeUpperBound. +func mergeLowerBound(dr *plan.DistRange, bound *plan.Expr, boundType plan.BoundType) bool { + if dr.LowerBoundType == plan.BoundType_UNBOUNDED { + dr.LowerBoundType = boundType + dr.LowerBound = bound + return true + } + newVal, ok1 := distBoundLiteralFloat64(bound) + curVal, ok2 := distBoundLiteralFloat64(dr.LowerBound) + if !ok1 || !ok2 { + return false + } + if newVal > curVal || (newVal == curVal && boundType == plan.BoundType_EXCLUSIVE) { + dr.LowerBoundType = boundType + dr.LowerBound = bound + } + return true +} + // peelAndRewriteDistFnFilters scans `filters` for predicates of shape // `origFuncName(col[partPos], vecLit) OP K` and, for each match: // diff --git a/pkg/sql/plan/apply_indices_vector_test.go b/pkg/sql/plan/apply_indices_vector_test.go index 9423ee26bf09c..dd7959427b1ef 100644 --- a/pkg/sql/plan/apply_indices_vector_test.go +++ b/pkg/sql/plan/apply_indices_vector_test.go @@ -330,24 +330,45 @@ func TestGetDistRangeFromFilters_NonMatching(t *testing.T) { require.Nil(t, dr) } -func TestGetDistRangeFromFiltersKeepsDuplicateSideAsResidual(t *testing.T) { +// Multiple same-side distance bounds must fold into the tightest bound (the +// intersection), independent of filter order, so the index enforces the correct +// range and does not depend on which predicate happens to appear first. See +// issue #25639. +func TestGetDistRangeFromFiltersKeepsTightestBound(t *testing.T) { const scanTag int32 = 11 const partPos int32 = 1 const vecVal = "[1,2,3]" vecLitArg := &plan.Expr{ Expr: &plan.Expr_Lit{Lit: &plan.Literal{Value: &plan.Literal_VecVal{VecVal: vecVal}}}, } - first := makeDistFnFilter(">", "l2_distance", scanTag, partPos, vecVal, f32Lit(1)) - second := makeDistFnFilter(">=", "l2_distance", scanTag, partPos, vecVal, f32Lit(10)) - var builder *QueryBuilder - remaining, distRange := builder.getDistRangeFromFilters( - []*plan.Expr{first, second}, partPos, "l2_distance", vecLitArg, - ) - require.Equal(t, plan.BoundType_EXCLUSIVE, distRange.LowerBoundType) - require.Equal(t, first.GetF().Args[1], distRange.LowerBound) - require.Equal(t, []*plan.Expr{second}, remaining) + // Two lower bounds: `> 1 AND >= 10` is `>= 10`; keep the tighter (larger). + { + loose := makeDistFnFilter(">", "l2_distance", scanTag, partPos, vecVal, f32Lit(1)) + tight := makeDistFnFilter(">=", "l2_distance", scanTag, partPos, vecVal, f32Lit(10)) + remaining, distRange := builder.getDistRangeFromFilters( + []*plan.Expr{loose, tight}, partPos, "l2_distance", vecLitArg, + ) + require.Empty(t, remaining) + require.Equal(t, plan.BoundType_INCLUSIVE, distRange.LowerBoundType) + require.Equal(t, tight.GetF().Args[1], distRange.LowerBound) + } + + // Two upper bounds, both orders: `< 1.1 AND < 1.2` is `< 1.1`; the tighter + // (smaller) upper bound wins regardless of input order (the #25639 case). + for _, order := range [][2]float32{{1.1, 1.2}, {1.2, 1.1}} { + fa := makeDistFnFilter("<", "l2_distance", scanTag, partPos, vecVal, f32Lit(order[0])) + fb := makeDistFnFilter("<", "l2_distance", scanTag, partPos, vecVal, f32Lit(order[1])) + remaining, distRange := builder.getDistRangeFromFilters( + []*plan.Expr{fa, fb}, partPos, "l2_distance", vecLitArg, + ) + require.Empty(t, remaining) + require.Equal(t, plan.BoundType_EXCLUSIVE, distRange.UpperBoundType) + v, ok := distBoundLiteralFloat64(distRange.UpperBound) + require.True(t, ok) + require.InDelta(t, 1.1, v, 1e-6) + } } func TestPeelAndRewriteDistFnFilters_AllOps(t *testing.T) { diff --git a/pkg/sql/plan/stats.go b/pkg/sql/plan/stats.go index 70e6d982f874d..52d84cdffa633 100644 --- a/pkg/sql/plan/stats.go +++ b/pkg/sql/plan/stats.go @@ -16,10 +16,11 @@ package plan import ( "bytes" + "cmp" "context" "fmt" "math" - "sort" + "slices" "strconv" "strings" "sync/atomic" @@ -1069,18 +1070,18 @@ func sortFilterListByStats(ctx context.Context, nodeID int32, builder *QueryBuil switch node.NodeType { case plan.Node_TABLE_SCAN: if node.ObjRef != nil && len(node.FilterList) >= 1 { - // NB: intentionally NOT migrated to slices.SortFunc. Filter evaluation - // order is semantically significant here (it decides which predicate a - // vector/IVF index probes with and which value a runtime error reports - // -- see operator/is_not_operator and vector/vector_ivf_mode, and the - // underlying IVF issue #25639). The comparator is non-strict (returns - // true on equal cost) and its exact tie ordering is an artifact of - // sort.Slice that no strict slices comparator reproduces, so this site - // is left on sort.Slice. - sort.Slice(node.FilterList, func(i, j int) bool { - cost1 := estimateFilterWeight(node.FilterList[i], 0) * node.FilterList[i].Selectivity - cost2 := estimateFilterWeight(node.FilterList[j], 0) * node.FilterList[j].Selectivity - return cost1 <= cost2 + // Use a STABLE sort with a strict comparator, deliberately not a plain + // (unstable) sort. Filter evaluation order is semantically significant + // here: it decides which predicate a vector/IVF index probes with (see + // #25639) and which value a runtime bool error reports. A stable sort + // keeps equal-cost filters in their original, as-written relative order, + // which is a deterministic and well-defined tie order; an unstable sort + // would reorder equal-cost filters arbitrarily and make those results + // depend on the sort implementation's internals. + slices.SortStableFunc(node.FilterList, func(a, b *plan.Expr) int { + cost1 := estimateFilterWeight(a, 0) * a.Selectivity + cost2 := estimateFilterWeight(b, 0) * b.Selectivity + return cmp.Compare(cost1, cost2) }) } } diff --git a/test/distributed/cases/operator/is_not_operator.result b/test/distributed/cases/operator/is_not_operator.result index 7cbdfa948eec9..3a32a2834c98e 100644 --- a/test/distributed/cases/operator/is_not_operator.result +++ b/test/distributed/cases/operator/is_not_operator.result @@ -33,7 +33,7 @@ invalid input: ' ' is not a valid bool expression SELECT * FROM is_test WHERE str1 IS FALSE; invalid input: '' is not a valid bool expression SELECT * FROM is_test WHERE str2 IS TRUE AND str1 IS TRUE; -invalid input: '' is not a valid bool expression +invalid input: ' ' is not a valid bool expression SELECT * FROM is_test WHERE LENGTH(str2) IS TRUE; str1 str2 d1 d2 d3 null null null diff --git a/test/distributed/cases/optimizer/associative.result b/test/distributed/cases/optimizer/associative.result index 4e767ff28a3e5..e4b84bf1c6a62 100644 --- a/test/distributed/cases/optimizer/associative.result +++ b/test/distributed/cases/optimizer/associative.result @@ -41,7 +41,7 @@ Project 𝄀 -> Table Scan on associative.file 𝄀 Filter Cond: (f.table_id = 1) 𝄀 -> Table Scan on associative.connector_job 𝄀 - Filter Cond: (job.status <> 5), (job.status <> 4) + Filter Cond: (job.status <> 4), (job.status <> 5) CREATE TABLE `lineitem` ( `L_ORDERKEY` bigint NOT NULL, `L_PARTKEY` int NOT NULL, diff --git a/test/distributed/cases/optimizer/in_domain.result b/test/distributed/cases/optimizer/in_domain.result index c06d8c5491b63..be48b143a9c72 100644 --- a/test/distributed/cases/optimizer/in_domain.result +++ b/test/distributed/cases/optimizer/in_domain.result @@ -51,7 +51,7 @@ explain select a from t1 where a in (1,2,3,4) and (a not in (2,3) or a = 99); TP QUERY PLAN Project -> Table Scan on d_indomain.t1 - Filter Cond: t1.a in ([1 4 99]), t1.a in ([1 2 3 4]) + Filter Cond: t1.a in ([1 2 3 4]), t1.a in ([1 4 99]) select a from t1 where a in (1,2,3,4,5,6,7,8) and a between 3 and 6 order by a; a 3 @@ -62,7 +62,7 @@ explain select a from t1 where a in (1,2,3,4,5,6,7,8) and a between 3 and 6; TP QUERY PLAN Project -> Table Scan on d_indomain.t1 - Filter Cond: t1.a BETWEEN 3 AND 6, t1.a in ([1 2 3 4 5 6 7 8]) + Filter Cond: t1.a in ([1 2 3 4 5 6 7 8]), t1.a BETWEEN 3 AND 6 insert into t1 values (101, 2); insert into t1 values (102, 2); select a,b from t1 where a in (1,101,102) and b != 2 order by a; @@ -72,7 +72,7 @@ explain select a,b from t1 where a in (1,101,102) and b != 2; TP QUERY PLAN Project -> Table Scan on d_indomain.t1 - Filter Cond: (t1.b <> 2), t1.a in ([1 101 102]) + Filter Cond: t1.a in ([1 101 102]), (t1.b <> 2) delete from t1 where a in (101, 102); create table t2(a int, b int); insert into t2 values (1,1),(2,2),(null,3); @@ -92,7 +92,7 @@ explain select a from t1 where a in (1,2,3,4) and (a <> 2 or a = 99); TP QUERY PLAN Project -> Table Scan on d_indomain.t1 - Filter Cond: t1.a in ([1 3 4 99]), t1.a in ([1 2 3 4]) + Filter Cond: t1.a in ([1 2 3 4]), t1.a in ([1 3 4 99]) select a from t1 where a in (1,2,3,4) and ((a not in (2,3) and a <> 3) or a = 99) order by a; a 1 @@ -101,7 +101,7 @@ explain select a from t1 where a in (1,2,3,4) and ((a not in (2,3) and a <> 3) o TP QUERY PLAN Project -> Table Scan on d_indomain.t1 - Filter Cond: t1.a in ([1 4 99]), t1.a in ([1 2 3 4]) + Filter Cond: t1.a in ([1 2 3 4]), t1.a in ([1 4 99]) select a from t1 where a in (1,2,3,4) and a not in (2, null) order by a; a select a from t1 where a in (1,2,3,4) and (a not in (2, null) or a = 99) order by a; @@ -110,7 +110,7 @@ explain select a from t1 where a in (1,2,3,4) and a not in (2, null); TP QUERY PLAN Project -> Table Scan on d_indomain.t1 - Filter Cond: t1.a in ([1 3 4]), (t1.a != (null)) + Filter Cond: (t1.a != (null)), t1.a in ([1 3 4]) create table t3 ( code varchar(255) not null, id bigint not null auto_increment, @@ -140,6 +140,6 @@ or (code is not null and code not in (5001010000, 5001020000, 6401010000))); TP QUERY PLAN Project -> Table Scan on d_indomain.t3 - Filter Cond: (((prefix_in(t3.__mo_cpkey_col) or prefix_eq(t3.__mo_cpkey_col)) and (t3.bveh = 'TFUA1000')) or (prefix_eq(t3.__mo_cpkey_col) and (t3.code IS NOT NULL))), prefix_in(t3.__mo_cpkey_col) + Filter Cond: prefix_in(t3.__mo_cpkey_col), (((prefix_in(t3.__mo_cpkey_col) or prefix_eq(t3.__mo_cpkey_col)) and (t3.bveh = 'TFUA1000')) or (prefix_eq(t3.__mo_cpkey_col) and (t3.code IS NOT NULL))) Block Filter Cond: prefix_in(t3.__mo_cpkey_col), t3.code in ([4999999010 5001010000 5001020000 6401010000]) drop database if exists d_indomain; diff --git a/test/distributed/cases/vector/vector_ivf_mode.sql b/test/distributed/cases/vector/vector_ivf_mode.sql index 14f54a7a76b3a..9c0c354e35487 100644 --- a/test/distributed/cases/vector/vector_ivf_mode.sql +++ b/test/distributed/cases/vector/vector_ivf_mode.sql @@ -83,8 +83,9 @@ WITH q AS (SELECT id, text, l2_distance(vec, '[0.1,-0.2,0.3,0.4,-0.1,0.2,0.0,0.5 -- Test Case: mode=post with OFFSET WITH q AS (SELECT id, text, l2_distance(vec, '[0.1,-0.2,0.3,0.4,-0.1,0.2,0.0,0.5]') AS dist FROM mini_vector_data) SELECT * FROM q ORDER BY dist LIMIT 3 OFFSET 2 by rank with option 'mode=post'; --- A one-sided reader range must not dereference the unbounded side, and a --- second upper bound must remain as a residual filter instead of replacing it. +-- A one-sided reader range must not dereference the unbounded side, and two +-- upper bounds must fold into the tightest bound (< 1.1), independent of order, +-- so the index enforces the intersection instead of the first predicate seen. SELECT id FROM mini_vector_data WHERE l2_distance(vec, '[0.1,-0.2,0.3,0.4,-0.1,0.2,0.0,0.5]') < 1.1 AND l2_distance(vec, '[0.1,-0.2,0.3,0.4,-0.1,0.2,0.0,0.5]') < 1.2 From 082d0111041a854163cd18a489149578fdcc64d4 Mon Sep 17 00:00:00 2001 From: fengttt Date: Mon, 13 Jul 2026 21:30:37 -0700 Subject: [PATCH 4/6] fix(review): validate distance bounds, dedupe literal decoder, revert non-transitive join sort Address review on the vector/DistRange changes: - Validate the FIRST distance bound. mergeUpperBound/mergeLowerBound accepted the first matching bound unconditionally; a non-literal first bound (column, param, cast) was peeled off the filter list but later rejected by the reader, silently dropping the constraint. They now validate every bound (including the first) and return false for non-literals so the predicate is kept as a residual filter. Added a first-invalid + second-valid test in both orders. - Single owner for the literal->float64 decode. Extract the decoder (with a nil guard) to plan.GetLiteralFloat64 in pkg/pb/plan; the planner (DistRange folding) and readutil.getLiteralFloat64 now both delegate to it, so plan-time and runtime bound handling can't drift. - getDistRangeFromFilters returns nil (not an empty DistRange) when every matching predicate was non-literal. - join_order.go: revert both compareStats sorts to sort.Slice. compareStats is non-transitive (0.01 selectivity tolerance), so the two-call wrapper is not a valid slices.SortFunc strict weak ordering. Tracked in #25702. - checkpoint/snapshot.go: guard the entries[i].end dereference with a nil check; nil entries now sort first, so the maxGlobalEnd scan must not assume non-nil. Verified: go build ./..., go vet, gofmt clean; unit tests pass; BVT vector, optimizer, join, dml/{select,delete,insert} all 100%. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/pb/plan/plan.go | 39 ++++++++++++ pkg/sql/plan/apply_indices_vector.go | 69 ++++++++------------- pkg/sql/plan/apply_indices_vector_test.go | 26 +++++++- pkg/sql/plan/join_order.go | 28 ++++----- pkg/vm/engine/readutil/reader.go | 30 +-------- pkg/vm/engine/tae/db/checkpoint/snapshot.go | 5 +- 6 files changed, 108 insertions(+), 89 deletions(-) diff --git a/pkg/pb/plan/plan.go b/pkg/pb/plan/plan.go index 1803a9d4529db..61abfe7f672be 100644 --- a/pkg/pb/plan/plan.go +++ b/pkg/pb/plan/plan.go @@ -115,3 +115,42 @@ func (m *ColRef) ColRefString() string { return string(data) } + +// GetLiteralFloat64 extracts a comparable float64 from a plain numeric literal +// expression, returning ok=false for anything that is not a non-null numeric +// literal (nil, non-literal, null, or a non-numeric literal such as a vector). +// It is the single owner of the "distance bound / literal -> float64" decode +// shared by the planner (DistRange folding) and the reader (index range setup). +func GetLiteralFloat64(expr *Expr) (float64, bool) { + if expr == nil { + return 0, false + } + lit := expr.GetLit() + if lit == nil || lit.Isnull { + return 0, false + } + switch v := lit.Value.(type) { + case *Literal_Fval: + return float64(v.Fval), true + case *Literal_Dval: + return v.Dval, true + case *Literal_I8Val: + return float64(v.I8Val), true + case *Literal_I16Val: + return float64(v.I16Val), true + case *Literal_I32Val: + return float64(v.I32Val), true + case *Literal_I64Val: + return float64(v.I64Val), true + case *Literal_U8Val: + return float64(v.U8Val), true + case *Literal_U16Val: + return float64(v.U16Val), true + case *Literal_U32Val: + return float64(v.U32Val), true + case *Literal_U64Val: + return float64(v.U64Val), true + default: + return 0, false + } +} diff --git a/pkg/sql/plan/apply_indices_vector.go b/pkg/sql/plan/apply_indices_vector.go index eccf5766b3e51..d66106b141afd 100644 --- a/pkg/sql/plan/apply_indices_vector.go +++ b/pkg/sql/plan/apply_indices_vector.go @@ -767,57 +767,37 @@ func (builder *QueryBuilder) getDistRangeFromFilters( currIdx++ } - return filters[:currIdx], distRange -} - -// distBoundLiteralFloat64 extracts a comparable float64 from a distance-bound -// literal expression. It accepts the same numeric literal kinds the index reader -// accepts (getLiteralFloat64), returning ok=false for anything that is not a -// plain numeric literal. -func distBoundLiteralFloat64(expr *plan.Expr) (float64, bool) { - lit := expr.GetLit() - if lit == nil || lit.Isnull { - return 0, false - } - switch v := lit.Value.(type) { - case *plan.Literal_Fval: - return float64(v.Fval), true - case *plan.Literal_Dval: - return v.Dval, true - case *plan.Literal_I8Val: - return float64(v.I8Val), true - case *plan.Literal_I16Val: - return float64(v.I16Val), true - case *plan.Literal_I32Val: - return float64(v.I32Val), true - case *plan.Literal_I64Val: - return float64(v.I64Val), true - case *plan.Literal_U8Val: - return float64(v.U8Val), true - case *plan.Literal_U16Val: - return float64(v.U16Val), true - case *plan.Literal_U32Val: - return float64(v.U32Val), true - case *plan.Literal_U64Val: - return float64(v.U64Val), true - default: - return 0, false + // If every matching predicate was non-literal (kept as a residual), the range + // was allocated but never bounded; return nil so callers don't stash an empty + // DistRange on the index reader. + if distRange != nil && + distRange.LowerBoundType == plan.BoundType_UNBOUNDED && + distRange.UpperBoundType == plan.BoundType_UNBOUNDED { + distRange = nil } + + return filters[:currIdx], distRange } // mergeUpperBound folds a new upper bound into dr, keeping the tighter (smaller, // or exclusive on an equal value) bound so the range is the intersection of all -// upper bounds. It returns false when the bounds cannot be compared as numeric -// literals, in which case the caller keeps the predicate as a residual filter. +// upper bounds. The bound MUST be a numeric literal the index reader can +// evaluate; otherwise the predicate would be peeled off the filter list but the +// reader would later reject it and silently drop the constraint. It returns +// false for a non-literal bound (including the first one) so the caller keeps +// the predicate as a residual filter. func mergeUpperBound(dr *plan.DistRange, bound *plan.Expr, boundType plan.BoundType) bool { + newVal, ok := plan.GetLiteralFloat64(bound) + if !ok { + return false + } if dr.UpperBoundType == plan.BoundType_UNBOUNDED { dr.UpperBoundType = boundType dr.UpperBound = bound return true } - newVal, ok1 := distBoundLiteralFloat64(bound) - curVal, ok2 := distBoundLiteralFloat64(dr.UpperBound) - if !ok1 || !ok2 { + curVal, ok := plan.GetLiteralFloat64(dr.UpperBound) + if !ok { return false } if newVal < curVal || (newVal == curVal && boundType == plan.BoundType_EXCLUSIVE) { @@ -830,14 +810,17 @@ func mergeUpperBound(dr *plan.DistRange, bound *plan.Expr, boundType plan.BoundT // mergeLowerBound folds a new lower bound into dr, keeping the tighter (larger, // or exclusive on an equal value) bound. See mergeUpperBound. func mergeLowerBound(dr *plan.DistRange, bound *plan.Expr, boundType plan.BoundType) bool { + newVal, ok := plan.GetLiteralFloat64(bound) + if !ok { + return false + } if dr.LowerBoundType == plan.BoundType_UNBOUNDED { dr.LowerBoundType = boundType dr.LowerBound = bound return true } - newVal, ok1 := distBoundLiteralFloat64(bound) - curVal, ok2 := distBoundLiteralFloat64(dr.LowerBound) - if !ok1 || !ok2 { + curVal, ok := plan.GetLiteralFloat64(dr.LowerBound) + if !ok { return false } if newVal > curVal || (newVal == curVal && boundType == plan.BoundType_EXCLUSIVE) { diff --git a/pkg/sql/plan/apply_indices_vector_test.go b/pkg/sql/plan/apply_indices_vector_test.go index dd7959427b1ef..bca413a91eae9 100644 --- a/pkg/sql/plan/apply_indices_vector_test.go +++ b/pkg/sql/plan/apply_indices_vector_test.go @@ -365,7 +365,31 @@ func TestGetDistRangeFromFiltersKeepsTightestBound(t *testing.T) { ) require.Empty(t, remaining) require.Equal(t, plan.BoundType_EXCLUSIVE, distRange.UpperBoundType) - v, ok := distBoundLiteralFloat64(distRange.UpperBound) + v, ok := plan.GetLiteralFloat64(distRange.UpperBound) + require.True(t, ok) + require.InDelta(t, 1.1, v, 1e-6) + } + + // A non-literal bound must never be peeled into the range (the reader can't + // evaluate it): it stays a residual filter, in both orders, while a literal + // bound of the same side still becomes the range. Regression for the + // "first bound accepted without validation" case. + nonLit := func() *plan.Expr { + return &plan.Expr{Expr: &plan.Expr_Col{Col: &plan.ColRef{RelPos: scanTag, ColPos: 7}}} + } + for _, nonLitFirst := range []bool{true, false} { + bad := makeDistFnFilter("<", "l2_distance", scanTag, partPos, vecVal, nonLit()) + good := makeDistFnFilter("<", "l2_distance", scanTag, partPos, vecVal, f32Lit(1.1)) + input := []*plan.Expr{bad, good} + if !nonLitFirst { + input = []*plan.Expr{good, bad} + } + remaining, distRange := builder.getDistRangeFromFilters( + input, partPos, "l2_distance", vecLitArg, + ) + require.Equal(t, []*plan.Expr{bad}, remaining) // non-literal kept as residual + require.Equal(t, plan.BoundType_EXCLUSIVE, distRange.UpperBoundType) + v, ok := plan.GetLiteralFloat64(distRange.UpperBound) require.True(t, ok) require.InDelta(t, 1.1, v, 1e-6) } diff --git a/pkg/sql/plan/join_order.go b/pkg/sql/plan/join_order.go index 02d8b8cd6d854..5988325221100 100644 --- a/pkg/sql/plan/join_order.go +++ b/pkg/sql/plan/join_order.go @@ -16,7 +16,7 @@ package plan import ( "fmt" - "slices" + "sort" "strings" "github.com/matrixorigin/matrixone/pkg/catalog" @@ -246,14 +246,12 @@ func (builder *QueryBuilder) determineJoinOrder(nodeID int32) int32 { } } - slices.SortFunc(subTrees, func(a, b *plan.Node) int { - if compareStats(a.Stats, b.Stats) { - return -1 - } - if compareStats(b.Stats, a.Stats) { - return 1 - } - return 0 + // compareStats is non-transitive (it groups selectivities within a 0.01 + // tolerance, then compares outcnt), so it is not a valid strict weak ordering + // and can't be a slices.SortFunc comparator. Keep it on sort.Slice until the + // comparator is made transitive; see issue #25702. + sort.Slice(subTrees, func(i, j int) bool { + return compareStats(subTrees[i].Stats, subTrees[j].Stats) }) leafByTag := make(map[int32]int32) @@ -587,14 +585,10 @@ func (builder *QueryBuilder) buildSubJoinTree(vertices []*joinVertex, vid int32) builder.buildSubJoinTree(vertices, child) dimensions = append(dimensions, vertices[child]) } - slices.SortFunc(dimensions, func(a, b *joinVertex) int { - if compareStats(a.node.Stats, b.node.Stats) { - return -1 - } - if compareStats(b.node.Stats, a.node.Stats) { - return 1 - } - return 0 + // See the subTrees sort above: compareStats is non-transitive, so this stays + // on sort.Slice until #25702 makes it a valid strict weak ordering. + sort.Slice(dimensions, func(i, j int) bool { + return compareStats(dimensions[i].node.Stats, dimensions[j].node.Stats) }) for _, child := range dimensions { diff --git a/pkg/vm/engine/readutil/reader.go b/pkg/vm/engine/readutil/reader.go index 009d0f6187c6b..c184c320ca912 100644 --- a/pkg/vm/engine/readutil/reader.go +++ b/pkg/vm/engine/readutil/reader.go @@ -583,34 +583,10 @@ func validBoundType(boundType plan.BoundType) bool { boundType == plan.BoundType_EXCLUSIVE } +// getLiteralFloat64 delegates to plan.GetLiteralFloat64 (the single owner of the +// literal->float64 decode) so plan-time and runtime bound handling can't drift. func getLiteralFloat64(expr *plan.Expr) (float64, bool) { - if expr == nil || expr.GetLit() == nil || expr.GetLit().Isnull { - return 0, false - } - switch value := expr.GetLit().Value.(type) { - case *plan.Literal_Fval: - return float64(value.Fval), true - case *plan.Literal_Dval: - return value.Dval, true - case *plan.Literal_I8Val: - return float64(value.I8Val), true - case *plan.Literal_I16Val: - return float64(value.I16Val), true - case *plan.Literal_I32Val: - return float64(value.I32Val), true - case *plan.Literal_I64Val: - return float64(value.I64Val), true - case *plan.Literal_U8Val: - return float64(value.U8Val), true - case *plan.Literal_U16Val: - return float64(value.U16Val), true - case *plan.Literal_U32Val: - return float64(value.U32Val), true - case *plan.Literal_U64Val: - return float64(value.U64Val), true - default: - return 0, false - } + return plan.GetLiteralFloat64(expr) } func (r *reader) GetOrderBy() []*plan.OrderBySpec { diff --git a/pkg/vm/engine/tae/db/checkpoint/snapshot.go b/pkg/vm/engine/tae/db/checkpoint/snapshot.go index 6787928656c38..7eee8ae3e7cd5 100644 --- a/pkg/vm/engine/tae/db/checkpoint/snapshot.go +++ b/pkg/vm/engine/tae/db/checkpoint/snapshot.go @@ -241,7 +241,10 @@ func filterSnapshotEntries(entries []*CheckpointEntry, snapshot *types.TS) []*Ch if snapshot != nil && snapshot.Equal(&maxGlobalEnd) { // Find the global checkpoint with end == maxGlobalEnd for i := range entries { - if entries[i].end.Equal(&maxGlobalEnd) && + // nil entries sort first (see the comparator above), so guard the + // dereference here rather than assume a non-nil invariant. + if entries[i] != nil && + entries[i].end.Equal(&maxGlobalEnd) && entries[i].entryType == ET_Global { // Return only the global checkpoint, since snapshot ts == gckp.end return []*CheckpointEntry{entries[i]} From cfd173f97bb89115aa2b2ee2aa32e8242d9b541b Mon Sep 17 00:00:00 2001 From: fengttt Date: Mon, 13 Jul 2026 21:51:11 -0700 Subject: [PATCH 5/6] fix(plan): make compareStats transitive via 0.01 selectivity bucketing compareStats grouped selectivities within a sliding |s1-s2| < 0.01 window, which is not transitive (a~b and b~c does not imply a~c), so it was not a valid strict weak ordering and its two join_order sort sites had to stay on sort.Slice. Bucket selectivity onto a fixed 0.01 grid (floor(sel/0.01)) and compare lexicographically by (bucket, outcnt). This is a genuine strict weak ordering, so compareStats now returns a three-way int and the two join_order sorts migrate to slices.SortFunc (the direct boolean use becomes `compareStats(...) < 0`). Behavior is unchanged for the test suite: the grid only differs from the sliding window near 0.01 boundaries, and a sweep of the EXPLAIN- and join-heavy dirs (optimizer, join, cte, subquery, recursive_cte, view, distinct, window, ...) shows no plan or result changes. Added TestCompareStatsIsStrictWeakOrdering (the review counter-example is now cycle-free, plus a brute-force transitivity/antisymmetry check over a selectivity/outcnt grid). Refs #25702. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/sql/plan/join_order.go | 18 ++++++------------ pkg/sql/plan/stats.go | 26 +++++++++++++++++--------- pkg/sql/plan/stats_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/pkg/sql/plan/join_order.go b/pkg/sql/plan/join_order.go index 5988325221100..8513423ed464e 100644 --- a/pkg/sql/plan/join_order.go +++ b/pkg/sql/plan/join_order.go @@ -16,7 +16,7 @@ package plan import ( "fmt" - "sort" + "slices" "strings" "github.com/matrixorigin/matrixone/pkg/catalog" @@ -246,12 +246,8 @@ func (builder *QueryBuilder) determineJoinOrder(nodeID int32) int32 { } } - // compareStats is non-transitive (it groups selectivities within a 0.01 - // tolerance, then compares outcnt), so it is not a valid strict weak ordering - // and can't be a slices.SortFunc comparator. Keep it on sort.Slice until the - // comparator is made transitive; see issue #25702. - sort.Slice(subTrees, func(i, j int) bool { - return compareStats(subTrees[i].Stats, subTrees[j].Stats) + slices.SortFunc(subTrees, func(a, b *plan.Node) int { + return compareStats(a.Stats, b.Stats) }) leafByTag := make(map[int32]int32) @@ -565,7 +561,7 @@ func shouldChangeParent(self, currentParent, nextParent int32, vertices []*joinV } } // self is the biggest node - return compareStats(nextParentStats, currentParentStats) + return compareStats(nextParentStats, currentParentStats) < 0 } // buildSubJoinTree build sub- join tree for a fact table and all its dimension tables @@ -585,10 +581,8 @@ func (builder *QueryBuilder) buildSubJoinTree(vertices []*joinVertex, vid int32) builder.buildSubJoinTree(vertices, child) dimensions = append(dimensions, vertices[child]) } - // See the subTrees sort above: compareStats is non-transitive, so this stays - // on sort.Slice until #25702 makes it a valid strict weak ordering. - sort.Slice(dimensions, func(i, j int) bool { - return compareStats(dimensions[i].node.Stats, dimensions[j].node.Stats) + slices.SortFunc(dimensions, func(a, b *joinVertex) int { + return compareStats(a.node.Stats, b.node.Stats) }) for _, child := range dimensions { diff --git a/pkg/sql/plan/stats.go b/pkg/sql/plan/stats.go index 52d84cdffa633..a3f3c7d5bcf50 100644 --- a/pkg/sql/plan/stats.go +++ b/pkg/sql/plan/stats.go @@ -1914,15 +1914,23 @@ func (builder *QueryBuilder) hasRecursiveScan(node *plan.Node) bool { return false } -func compareStats(stats1, stats2 *Stats) bool { - // selectivity is first considered to reduce data - // when selectivity very close, we first join smaller table - if math.Abs(stats1.Selectivity-stats2.Selectivity) > 0.01 { - return stats1.Selectivity < stats2.Selectivity - } else { - // todo we need to calculate ndv of outcnt here - return stats1.Outcnt < stats2.Outcnt - } +// compareStats orders join candidates: smaller selectivity first (to reduce +// data), and when the selectivity is very close, smaller outcnt first (join the +// smaller table first). It returns a three-way result (negative when stats1 +// sorts before stats2). +// +// "Very close" is defined by bucketing selectivity onto a fixed 0.01 grid rather +// than a sliding |s1-s2| < 0.01 window. The window version is NOT transitive +// (a~b and b~c does not imply a~c), which makes it an invalid comparator for +// slices.SortFunc; the grid is a genuine strict weak ordering. See issue #25702. +func compareStats(stats1, stats2 *Stats) int { + b1 := int64(math.Floor(stats1.Selectivity / 0.01)) + b2 := int64(math.Floor(stats2.Selectivity / 0.01)) + if b1 != b2 { + return cmp.Compare(b1, b2) + } + // todo we need to calculate ndv of outcnt here + return cmp.Compare(stats1.Outcnt, stats2.Outcnt) } func andSelectivity(s1, s2 float64) float64 { diff --git a/pkg/sql/plan/stats_test.go b/pkg/sql/plan/stats_test.go index 205846c3599be..09f902ba97990 100644 --- a/pkg/sql/plan/stats_test.go +++ b/pkg/sql/plan/stats_test.go @@ -1458,3 +1458,41 @@ func TestGetExprNdv(t *testing.T) { require.Equal(t, -1.0, ndv) }) } + +// compareStats must be a valid strict weak ordering (transitive + antisymmetric) +// so it is safe to use with slices.SortFunc. See issue #25702. +func TestCompareStatsIsStrictWeakOrdering(t *testing.T) { + // The review counter-example that broke the old sliding-window comparator: + // A~B and B~C (within 0.01) but A and C fall in different buckets. With the + // 0.01-grid bucketing there is no cycle. + a := &Stats{Selectivity: 0.000, Outcnt: 3} + b := &Stats{Selectivity: 0.009, Outcnt: 2} + c := &Stats{Selectivity: 0.018, Outcnt: 1} + // a and b share selectivity bucket 0 -> compare outcnt (3 vs 2) -> b before a. + require.True(t, compareStats(b, a) < 0) + require.True(t, compareStats(a, b) > 0) + require.True(t, compareStats(b, c) < 0) // bucket 0 < bucket 1 + require.True(t, compareStats(a, c) < 0) // bucket 0 < bucket 1 + // consistent order b < a < c (the old comparator produced a b x Date: Tue, 14 Jul 2026 14:52:07 -0700 Subject: [PATCH 6/6] fix(plan): sort geometry coverage intervals by an exact total order parameterIntervalsCoverSegment sorted its intervals with a comparator that treated starts within 1e-9 (sameGeometryCoordinate) as equal and then compared ends. That epsilon band is non-transitive (start A~B and B~C does not imply A~C), so it is not a valid slices.SortFunc strict weak ordering: for a cyclic near-epsilon triple the sort can leave a non-minimum-start interval at index 0, and the coverage check then wrongly bails at intervals[0].start > 1e-9 even when the intervals together cover the whole segment -- changing LINESTRING/ MULTILINESTRING coverage predicate results. Sort by an exact total order on (start, end) instead; the epsilon tolerance stays in the gap/coverage loop, where it is applied pairwise and does not require transitivity. Behavior is unchanged for well-separated intervals (all real data); only the pathological near-epsilon ordering is corrected. Added TestParameterIntervalsCoverSegmentNearEpsilonBoundary: the reviewer's cyclic-triple boundary case (which returns a wrong false under the old comparator, verified) now reports full coverage as true in every input order, and a genuine gap still returns false. Verified: go build ./..., go vet, gofmt clean; the new unit test and the geo BVT suite pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/sql/plan/function/func_binary.go | 13 ++++++--- pkg/sql/plan/function/func_binary_test.go | 33 +++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index bff61a74090f1..d16e7dcf5cd01 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -10485,11 +10485,18 @@ func parameterIntervalsCoverSegment(intervals []geometryParamInterval) bool { return false } + // Sort by an exact total order on (start, end). Do NOT fold starts within an + // epsilon into an "equal" bucket here: an epsilon band is non-transitive + // (start A~B and B~C does not imply A~C), which violates slices.SortFunc's + // strict-weak-ordering contract and can leave a non-minimum-start interval at + // position 0 -- making the coverage check below wrongly bail on + // intervals[0].start > 1e-9. The epsilon tolerance stays in the gap/coverage + // loop, where it is applied pairwise and transitivity is not required. slices.SortFunc(intervals, func(a, b geometryParamInterval) int { - if sameGeometryCoordinate(a.start, b.start) { - return cmp.Compare(a.end, b.end) + if c := cmp.Compare(a.start, b.start); c != 0 { + return c } - return cmp.Compare(a.start, b.start) + return cmp.Compare(a.end, b.end) }) coveredEnd := intervals[0].end diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index cd686ca61d5ba..5043f95639b40 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -12932,3 +12932,36 @@ func TestEltCoversSignedAndSelectListPaths(t *testing.T) { testSelectList(t, types.T_uint64.ToType(), []uint64{1, 1}) }) } + +// Regression for the non-transitive interval sort comparator (see PR #25616). +// The three starts sit inside the 1e-9 epsilon band around 0 and, with the old +// epsilon comparator, form a cyclic order (A~B, B~C, but A!~C; ends give A>B, +// B>C, C>A) that could leave a non-minimum-start interval at position 0 and make +// parameterIntervalsCoverSegment wrongly bail at intervals[0].start > 1e-9. +// Together with [0.85, 1.0] the intervals cover the whole [0, 1] segment, so +// coverage must be reported true regardless of the input order. +func TestParameterIntervalsCoverSegmentNearEpsilonBoundary(t *testing.T) { + base := []geometryParamInterval{ + {start: 0, end: 0.9}, + {start: 0.9e-9, end: 0.8}, + {start: 1.8e-9, end: 0.7}, + {start: 0.85, end: 1.0}, + } + orders := [][]int{ + {0, 1, 2, 3}, + {2, 1, 0, 3}, // largest-epsilon start first + {3, 2, 1, 0}, + {1, 3, 2, 0}, + } + for _, ord := range orders { + in := make([]geometryParamInterval, len(ord)) + for i, idx := range ord { + in[i] = base[idx] + } + require.True(t, parameterIntervalsCoverSegment(in), "input order %v should still cover [0,1]", ord) + } + + // A genuine gap must still report false: [0,0.4] and [0.6,1.0] leave (0.4,0.6). + gap := []geometryParamInterval{{start: 0, end: 0.4}, {start: 0.6, end: 1.0}} + require.False(t, parameterIntervalsCoverSegment(gap)) +}