Skip to content
Draft
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
85171de
*: plumb per-column DESC flag through index metadata (phase 1 of #2519)
takaidohigasi Apr 24, 2026
e92b461
planner: honor IndexColumn.Desc when matching sort property (phase 2 …
takaidohigasi Apr 24, 2026
a23319b
codec: add EncodeKeyWithDesc/DecodeOneWithDesc primitives (phase 3a o…
takaidohigasi Apr 24, 2026
e414040
tablecodec: encode DESC index columns with complemented bytes (phase …
takaidohigasi Apr 24, 2026
686a9dc
distsql,executor,tables: wire DESC encoding through read paths (phase…
takaidohigasi Apr 24, 2026
adb81f0
codec: auto-detect DESC flag bytes in chunk Decoder (phase 3d-A of #2…
takaidohigasi Apr 24, 2026
a2407dd
planner,model: enforce IndexInfo.IsServable at plan time (#2519)
takaidohigasi Apr 25, 2026
b05e420
ddl: gate CREATE INDEX on TiKV cluster version (#2519)
takaidohigasi Apr 25, 2026
385d13f
planner: support mixed-direction composite indexes (#2519)
takaidohigasi Apr 25, 2026
82006d0
ddl: regression test for DESC on expression-index parts (#2519)
takaidohigasi Apr 25, 2026
7f506a5
ddl: lock down sysvar create-time-only semantics for DESC indexes (#2…
takaidohigasi Apr 25, 2026
be86ba2
ddl: clarify MinTiKVVersionForDescIndex is a release-coordinated TODO…
takaidohigasi Apr 25, 2026
61a14ea
ddl: reject DESC on the columns of a clustered PRIMARY KEY (#2519)
takaidohigasi Apr 25, 2026
f40e70d
executor: propagate IndexRangesToKVRangesWithDesc error in IndexMerge…
takaidohigasi Apr 25, 2026
3f246ba
planner: extend IsServable fence to LOAD DATA and IMPORT INTO (#2519)
takaidohigasi Apr 25, 2026
fcff441
ddl: bake tidb_enable_descending_index decision at SQL frontend (#2519)
takaidohigasi Apr 25, 2026
4ce9687
ddl: delay DESC preflight until after OnExistIgnore short-circuit (#2…
takaidohigasi Apr 25, 2026
3faacae
ddl: bound PD GetAllStores call with a 5s timeout (#2519)
takaidohigasi Apr 25, 2026
ffa8638
ddl: accept pre-release TiKV at the DESC index version floor (#2519)
takaidohigasi Apr 25, 2026
f89329e
planner: extend IsServable fence to writable non-public indexes (#2519)
takaidohigasi Apr 26, 2026
e9200da
ddl: clarify checkTiKVSupportsDescIndex docstring (#2519)
takaidohigasi Apr 26, 2026
49bbaca
ddl: assert idx_a presence in TestDescendingIndexSysvarIsCreateTimeOn…
takaidohigasi Apr 26, 2026
7fb5348
ddl: skip TiKV version gate for hypothetical DESC indexes (#2519)
takaidohigasi Apr 26, 2026
37f2288
planner: run IMPORT INTO servability fence on latest schema (#2519)
takaidohigasi Apr 26, 2026
8eec79a
tests: add tiup-playground e2e smoke test for descending-order indexe…
takaidohigasi Apr 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pkg/ddl/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,18 @@ func BuildTableInfo(
isSingleIntPK := isSingleIntPKFromTableInfo(constr, tbInfo)

if ShouldBuildClusteredIndex(ctx.GetClusteredIndexDefMode(), constr.Option, isSingleIntPK) {
// Reject DESC on the columns of a clustered PRIMARY KEY.
// For PKIsHandle (single-int PK) the column becomes the
// row's int handle directly and BuildIndexInfo is never
// invoked, so this guard catches that case. For
// IsCommonHandle the same check fires again inside
// BuildIndexInfo for defence-in-depth. See pingcap/tidb#2519.
for _, key := range constr.Keys {
if key.Desc {
return nil, errors.Errorf(
"DESC is not supported on the columns of a clustered PRIMARY KEY; either drop the DESC keyword or declare the primary key as NONCLUSTERED")
}
}
if isSingleIntPK {
tbInfo.PKIsHandle = true
} else {
Expand Down
370 changes: 370 additions & 0 deletions pkg/ddl/db_integration_test.go

Large diffs are not rendered by default.

125 changes: 125 additions & 0 deletions pkg/ddl/desc_index_version_check_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Copyright 2026 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package ddl

import (
"testing"

"github.com/pingcap/kvproto/pkg/metapb"
"github.com/pingcap/tidb/pkg/ddl/placement"
"github.com/stretchr/testify/require"
)

// store builds a synthetic *metapb.Store for the version-check tests.
// Pass an empty version to simulate a store that hasn't reported one yet.
func storeAt(id uint64, version string, isTiFlash bool) *metapb.Store {
s := &metapb.Store{Id: id, Version: version, Address: "mock"}
if isTiFlash {
s.Labels = []*metapb.StoreLabel{{Key: placement.EngineLabelKey, Value: placement.EngineLabelTiFlash}}
}
return s
}

func TestCheckStoresMeetDescIndexMinVersion(t *testing.T) {
const minVer = "9.0.0"
const failClosed = false // production semantics

t.Run("all TiKV stores are new enough", func(t *testing.T) {
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
storeAt(2, "9.1.0", false),
storeAt(3, "v9.0.0", false), // leading-v normalisation
}
require.NoError(t, checkStoresMeetDescIndexMinVersion(stores, minVer, failClosed))
})

t.Run("an old TiKV store fails the gate", func(t *testing.T) {
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
storeAt(2, "8.5.0", false),
}
err := checkStoresMeetDescIndexMinVersion(stores, minVer, failClosed)
require.Error(t, err)
require.Contains(t, err.Error(), "store 2")
require.Contains(t, err.Error(), "8.5.0")
require.Contains(t, err.Error(), minVer)
require.Contains(t, err.Error(), "upgrade TiKV")
})

t.Run("TiFlash stores are excluded from the check", func(t *testing.T) {
// A TiFlash store on an old version must not block the gate; the
// check is for TiKV only because TiKV alone runs the coprocessor
// path that needs the new decoder.
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
storeAt(2, "1.0.0", true), // ancient TiFlash, ignored
}
require.NoError(t, checkStoresMeetDescIndexMinVersion(stores, minVer, failClosed))
})

t.Run("tombstone stores are skipped", func(t *testing.T) {
old := storeAt(2, "8.0.0", false)
old.State = metapb.StoreState_Tombstone
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
old,
}
require.NoError(t, checkStoresMeetDescIndexMinVersion(stores, minVer, failClosed))
})

t.Run("empty version fails closed in production", func(t *testing.T) {
// A store that hasn't reported a clean semver yet cannot prove it
// can decode descending-order keys. The gate must reject the DDL
// rather than fall through and risk silent corruption. The operator
// can retry once PD has reconciled.
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
storeAt(2, "", false),
}
err := checkStoresMeetDescIndexMinVersion(stores, minVer, failClosed)
require.Error(t, err)
require.Contains(t, err.Error(), "store 2")
require.Contains(t, err.Error(), "has not reported a version")
})

t.Run("empty version is tolerated in tests", func(t *testing.T) {
// In `intest` builds the mock store fixture reports empty versions
// for every replica, so the gate must let DDL through to allow
// integration tests to run.
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
storeAt(2, "", false),
}
require.NoError(t, checkStoresMeetDescIndexMinVersion(stores, minVer, true))
})

t.Run("garbage version always fails closed", func(t *testing.T) {
// Unparsable version strings are unambiguously a bug or a
// misconfigured store; never tolerate them, even in tests.
stores := []*metapb.Store{
storeAt(1, "9.0.0", false),
storeAt(2, "not-a-semver", false),
}
err := checkStoresMeetDescIndexMinVersion(stores, minVer, true)
require.Error(t, err)
require.Contains(t, err.Error(), "store 2")
require.Contains(t, err.Error(), "unparsable")
})

t.Run("malformed minVersion is reported", func(t *testing.T) {
err := checkStoresMeetDescIndexMinVersion(nil, "definitely-not-semver", failClosed)
require.Error(t, err)
})
}
134 changes: 134 additions & 0 deletions pkg/ddl/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ import (
"time"
"unicode/utf8"

"github.com/coreos/go-semver/semver"
"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/kvproto/pkg/metapb"
"github.com/pingcap/tidb/pkg/config"
"github.com/pingcap/tidb/pkg/config/kerneltype"
"github.com/pingcap/tidb/pkg/ddl/label"
"github.com/pingcap/tidb/pkg/ddl/logutil"
"github.com/pingcap/tidb/pkg/ddl/placement"
"github.com/pingcap/tidb/pkg/ddl/resourcegroup"
sess "github.com/pingcap/tidb/pkg/ddl/session"
ddlutil "github.com/pingcap/tidb/pkg/ddl/util"
Expand Down Expand Up @@ -78,10 +81,26 @@ import (
"github.com/pingcap/tidb/pkg/util/traceevent"
"github.com/pingcap/tidb/pkg/util/tracing"
"github.com/tikv/client-go/v2/oracle"
"github.com/tikv/client-go/v2/tikv"
pdhttp "github.com/tikv/pd/client/http"
"go.uber.org/zap"
)

// MinTiKVVersionForDescIndex is the lowest TiKV semver that contains the
// DESC-aware coprocessor decoder added for pingcap/tidb#2519. The CREATE
// INDEX path refuses to persist a column with Desc=true unless every TiKV
// store reports at least this version, so the cluster can never end up in
// a state where TiDB writes complemented index bytes that TiKV mis-decodes.
//
// TODO(release): bump this in lockstep with the TiKV release that contains
// the matching Rust changes (split_datum / decode_one_with_desc auto-detect
// of complemented flag bytes). The placeholder below intentionally sits
// well past the most recent stable release so the gate fails closed in any
// pre-release deployment — operators who flip tidb_enable_descending_index
// on a cluster that hasn't been upgraded will see a clear "upgrade TiKV"
// error rather than silently corrupting their indexes.
const MinTiKVVersionForDescIndex = "9.0.0"

const (
expressionIndexPrefix = "_V$"
tableNotExist = -1
Expand Down Expand Up @@ -1088,6 +1107,21 @@ func (e *executor) createTableWithInfoJob(
return nil, infoschema.ErrDatabaseNotExists.GenWithStackByArgs(dbName)
}

// Run the descending-index TiKV-version gate at the shared chokepoint
// for CREATE TABLE so CreateTableWithInfo and BatchCreateTableWithInfo
// are covered alongside CreateTable. If any index in the about-to-be-
// persisted TableInfo carries Desc=true and the cluster contains a
// TiKV that cannot decode descending keys, fail early before the DDL
// job is queued. See pingcap/tidb#2519.
for _, idx := range tbInfo.Indices {
if idx.HasDescColumn() {
if err = checkTiKVSupportsDescIndex(ctx); err != nil {
return nil, errors.Trace(err)
}
break
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

if err = handleTablePlacement(ctx, tbInfo); err != nil {
return nil, errors.Trace(err)
}
Expand Down Expand Up @@ -4703,6 +4737,11 @@ func (e *executor) CreatePrimaryKey(ctx sessionctx.Context, ti ast.Ident, indexN
if err != nil {
return errors.Trace(err)
}
if hasDescIndexColumn(indexColumns) {
if err := checkTiKVSupportsDescIndex(ctx); err != nil {
return err
}
}
if _, err = CheckPKOnGeneratedColumn(tblInfo, indexPartSpecifications); err != nil {
return err
}
Expand Down Expand Up @@ -4943,6 +4982,95 @@ func (e *executor) CreateIndex(ctx sessionctx.Context, stmt *ast.CreateIndexStmt
stmt.IndexPartSpecifications, stmt.IndexOption, stmt.IfNotExists)
}

// hasDescIndexColumn reports whether any built IndexColumn carries Desc=true.
func hasDescIndexColumn(cols []*model.IndexColumn) bool {
for _, c := range cols {
if c.Desc {
return true
}
}
return false
}

// checkTiKVSupportsDescIndex returns an error if any non-TiFlash store in
// the cluster runs a TiKV version below MinTiKVVersionForDescIndex. Called
// when CREATE INDEX would persist a column with Desc=true. Mock stores
// (which never satisfy tikv.Storage) silently pass — the auto-detection
// path in the unistore coprocessor handles them.
func checkTiKVSupportsDescIndex(ctx sessionctx.Context) error {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
tikvStore, ok := ctx.GetStore().(tikv.Storage)
if !ok {
return nil
}
pdCli := tikvStore.GetRegionCache().PDClient()
if pdCli == nil {
return nil
}
stores, err := pdCli.GetAllStores(context.Background())
if err != nil {
return errors.Annotate(err, "checking TiKV cluster version for descending index")
}
// In production we fail closed on missing versions. The mock store
// fixture used in `intest` builds reports empty versions for every
// replica; tolerate that case so integration tests can exercise
// descending-index DDL without spinning up a real PD.
return checkStoresMeetDescIndexMinVersion(stores, MinTiKVVersionForDescIndex, intest.InTest)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

// checkStoresMeetDescIndexMinVersion is the pure version-comparison core of
// checkTiKVSupportsDescIndex, split out so the caller can pass a mocked
// store list in unit tests without spinning up a PD client.
//
// tolerateMissingVersion controls how to treat stores that report an empty
// version string. The production caller passes false (fail closed); the
// in-process test caller passes true so the mock-store fixture, which never
// fills in a version, doesn't block descending-index DDL.
// Garbage (unparsable) version strings always fail closed because they are
// unambiguously a misconfiguration, never a fixture artifact.
func checkStoresMeetDescIndexMinVersion(stores []*metapb.Store, minVersion string, tolerateMissingVersion bool) error {
required, err := semver.NewVersion(minVersion)
if err != nil {
return errors.Trace(err)
}
for _, s := range stores {
if s.GetState() == metapb.StoreState_Tombstone {
continue
}
if storeIsTiFlash(s) {
continue
}
if s.Version == "" {
if tolerateMissingVersion {
continue
}
return errors.Errorf(
"cannot create descending-order index: TiKV store %d at %s has not reported a version yet; retry after the cluster has reported its store versions",
s.Id, s.Address)
}
actual, parseErr := semver.NewVersion(strings.TrimPrefix(s.Version, "v"))
if parseErr != nil {
return errors.Annotatef(parseErr,
"cannot create descending-order index: TiKV store %d at %s reports an unparsable version %q",
s.Id, s.Address, s.Version)
}
if actual.LessThan(*required) {
return errors.Errorf(
"cannot create descending-order index: TiKV store %d at %s reports version %s, which is below the minimum %s required by this feature; upgrade TiKV before retrying",
s.Id, s.Address, s.Version, minVersion)
}
}
return nil
}

func storeIsTiFlash(store *metapb.Store) bool {
for _, label := range store.Labels {
if label.Key == placement.EngineLabelKey && label.Value == placement.EngineLabelTiFlash {
return true
}
}
return false
}

// addHypoIndexIntoCtx adds this index as a hypo-index into this ctx.
func (*executor) addHypoIndexIntoCtx(ctx sessionctx.Context, schemaName, tableName ast.CIStr, indexInfo *model.IndexInfo) error {
sctx := ctx.GetSessionVars()
Expand Down Expand Up @@ -5009,6 +5137,12 @@ func (e *executor) createIndex(ctx sessionctx.Context, ti ast.Ident, keyType ast
return errors.Trace(err)
}

if hasDescIndexColumn(indexColumns) {
if err := checkTiKVSupportsDescIndex(ctx); err != nil {
return err
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

if err = checkCreateGlobalIndex(ctx.GetSessionVars().StmtCtx.ErrCtx(), tblInfo, indexName.O, indexColumns, unique, indexOption != nil && indexOption.Global); err != nil {
return err
}
Expand Down
28 changes: 28 additions & 0 deletions pkg/ddl/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ func buildIndexColumns(ctx *metabuild.Context, columns []*model.ColumnInfo, inde
maxIndexLength := config.GetGlobalConfig().MaxIndexLength
// The sum of length of all index columns.
sumLength := 0
descEnabled := vardef.EnableDescendingIndex.Load()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
for _, ip := range indexPartSpecifications {
col = model.FindColumnInfo(columns, ip.Column.Name.L)
if col == nil {
Expand All @@ -132,6 +133,11 @@ func buildIndexColumns(ctx *metabuild.Context, columns []*model.ColumnInfo, inde
if columnarIndexType == model.ColumnarIndexTypeFulltext && !types.IsString(col.FieldType.GetType()) {
return nil, false, dbterror.ErrUnsupportedAddColumnarIndex.FastGenByArgs(fmt.Sprintf("only support string type, but this is type: %s", col.FieldType.String()))
}
// Columnar indexes cannot support descending order; reject explicitly.
// Fulltext also has its own DESC check in buildFullTextInfoWithCheck.
if ip.Desc && columnarIndexType != model.ColumnarIndexTypeNA {
return nil, false, dbterror.ErrUnsupportedAddColumnarIndex.FastGenByArgs(fmt.Sprintf("%s does not support DESC order", columnarIndexType.SQLName()))
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// return error in strict sql mode
if columnarIndexType == model.ColumnarIndexTypeNA {
Expand Down Expand Up @@ -175,10 +181,14 @@ func buildIndexColumns(ctx *metabuild.Context, columns []*model.ColumnInfo, inde
ctx.AppendWarning(dbterror.ErrTooLongKey.FastGenByArgs(sumLength, maxIndexLength))
}

// When the feature gate is off, preserve historical MySQL 5.7 behavior:
// DESC is accepted by the parser but silently ignored. This keeps legacy
// migration scripts working unchanged.
idxParts = append(idxParts, &model.IndexColumn{
Name: col.Name,
Offset: col.Offset,
Length: indexColLen,
Desc: ip.Desc && descEnabled,
})
}

Expand Down Expand Up @@ -431,6 +441,24 @@ func BuildIndexInfo(
if err != nil {
return nil, errors.Trace(err)
}
// Reject DESC on a clustered PRIMARY KEY. For PKIsHandle the PK column is
// the row's int handle itself (encoded ascending and never going through
// the DESC-aware index encoder), and for IsCommonHandle the PK columns
// determine the row-key byte order, which the common-handle range builder
// does not yet encode with per-column DESC awareness. Allowing DESC here
// would silently miscompare row keys, so error out at DDL time. A
// NONCLUSTERED primary key is encoded just like any unique secondary
// index and supports DESC normally.
if isPrimary && idxInfo.HasDescColumn() && (tblInfo.IsCommonHandle || tblInfo.PKIsHandle) {
return nil, errors.Errorf(
"DESC is not supported on the columns of a clustered PRIMARY KEY; either drop the DESC keyword or declare the primary key as NONCLUSTERED")
}
// Bump the index-metadata version when any descending column is present so
// that an older TiDB reading this schema refuses to serve queries against
// the index rather than returning wrong results — see IndexInfo.IsServable.
if idxInfo.HasDescColumn() {
idxInfo.Version = 1
}

if indexOption != nil {
idxInfo.Comment = indexOption.Comment
Expand Down
Loading
Loading