Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executor: append warning for sorting non-scalar json value #37544

Closed
wants to merge 1 commit into from
Closed

executor: append warning for sorting non-scalar json value #37544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 10 additions & 10 deletions executor/aggfuncs/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,23 @@ func Build(ctx sessionctx.Context, aggFuncDesc *aggregation.AggFuncDesc, ordinal
func BuildWindowFunctions(ctx sessionctx.Context, windowFuncDesc *aggregation.AggFuncDesc, ordinal int, orderByCols []*expression.Column) AggFunc {
switch windowFuncDesc.Name {
case ast.WindowFuncRank:
return buildRank(ordinal, orderByCols, false)
return buildRank(ctx, ordinal, orderByCols, false)
case ast.WindowFuncDenseRank:
return buildRank(ordinal, orderByCols, true)
return buildRank(ctx, ordinal, orderByCols, true)
case ast.WindowFuncRowNumber:
return buildRowNumber(windowFuncDesc, ordinal)
case ast.WindowFuncFirstValue:
return buildFirstValue(windowFuncDesc, ordinal)
case ast.WindowFuncLastValue:
return buildLastValue(windowFuncDesc, ordinal)
case ast.WindowFuncCumeDist:
return buildCumeDist(ordinal, orderByCols)
return buildCumeDist(ctx, ordinal, orderByCols)
case ast.WindowFuncNthValue:
return buildNthValue(windowFuncDesc, ordinal)
case ast.WindowFuncNtile:
return buildNtile(windowFuncDesc, ordinal)
case ast.WindowFuncPercentRank:
return buildPercentRank(ordinal, orderByCols)
return buildPercentRank(ctx, ordinal, orderByCols)
case ast.WindowFuncLead:
return buildLead(ctx, windowFuncDesc, ordinal)
case ast.WindowFuncLag:
Expand Down Expand Up @@ -635,11 +635,11 @@ func buildRowNumber(aggFuncDesc *aggregation.AggFuncDesc, ordinal int) AggFunc {
return &rowNumber{base}
}

func buildRank(ordinal int, orderByCols []*expression.Column, isDense bool) AggFunc {
func buildRank(ctx sessionctx.Context, ordinal int, orderByCols []*expression.Column, isDense bool) AggFunc {
base := baseAggFunc{
ordinal: ordinal,
}
r := &rank{baseAggFunc: base, isDense: isDense, rowComparer: buildRowComparer(orderByCols)}
r := &rank{baseAggFunc: base, isDense: isDense, rowComparer: buildRowComparer(ctx, orderByCols)}
return r
}

Expand All @@ -659,11 +659,11 @@ func buildLastValue(aggFuncDesc *aggregation.AggFuncDesc, ordinal int) AggFunc {
return &lastValue{baseAggFunc: base, tp: aggFuncDesc.RetTp}
}

func buildCumeDist(ordinal int, orderByCols []*expression.Column) AggFunc {
func buildCumeDist(ctx sessionctx.Context, ordinal int, orderByCols []*expression.Column) AggFunc {
base := baseAggFunc{
ordinal: ordinal,
}
r := &cumeDist{baseAggFunc: base, rowComparer: buildRowComparer(orderByCols)}
r := &cumeDist{baseAggFunc: base, rowComparer: buildRowComparer(ctx, orderByCols)}
return r
}

Expand All @@ -686,11 +686,11 @@ func buildNtile(aggFuncDes *aggregation.AggFuncDesc, ordinal int) AggFunc {
return &ntile{baseAggFunc: base, n: n}
}

func buildPercentRank(ordinal int, orderByCols []*expression.Column) AggFunc {
func buildPercentRank(ctx sessionctx.Context, ordinal int, orderByCols []*expression.Column) AggFunc {
base := baseAggFunc{
ordinal: ordinal,
}
return &percentRank{baseAggFunc: base, rowComparer: buildRowComparer(orderByCols)}
return &percentRank{baseAggFunc: base, rowComparer: buildRowComparer(ctx, orderByCols)}
}

func buildLeadLag(ctx sessionctx.Context, aggFuncDesc *aggregation.AggFuncDesc, ordinal int) baseLeadLag {
Expand Down
8 changes: 5 additions & 3 deletions executor/aggfuncs/func_rank.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,18 @@ type rowComparer struct {
colIdx []int
}

func buildRowComparer(cols []*expression.Column) rowComparer {
func buildRowComparer(ctx sessionctx.Context, cols []*expression.Column) rowComparer {
wp := ctx.GetSessionVars().StmtCtx.AppendWarning

rc := rowComparer{}
rc.colIdx = make([]int, 0, len(cols))
rc.cmpFuncs = make([]chunk.CompareFunc, 0, len(cols))
for _, col := range cols {
cmpFunc := chunk.GetCompareFunc(col.RetType)
cmpFunc := chunk.GetCompareFunc(col.RetType, wp)
if cmpFunc == nil {
continue
}
rc.cmpFuncs = append(rc.cmpFuncs, chunk.GetCompareFunc(col.RetType))
rc.cmpFuncs = append(rc.cmpFuncs, chunk.GetCompareFunc(col.RetType, wp))
rc.colIdx = append(rc.colIdx, col.Index)
}
return rc
Expand Down
2 changes: 1 addition & 1 deletion executor/sort.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (e *SortExec) initCompareFuncs() {
e.keyCmpFuncs = make([]chunk.CompareFunc, len(e.ByItems))
for i := range e.ByItems {
keyType := e.ByItems[i].Expr.GetType()
e.keyCmpFuncs[i] = chunk.GetCompareFunc(keyType)
e.keyCmpFuncs[i] = chunk.GetCompareFunc(keyType, e.ctx.GetSessionVars().StmtCtx.AppendWarning)
}
}

Expand Down
13 changes: 13 additions & 0 deletions expression/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7471,3 +7471,16 @@ func TestCompareJSONWithOtherType(t *testing.T) {
tk.MustQuery("select * from t where a < 6;").Check(testkit.Rows("5"))
tk.MustQuery("select * from t where a > 5;").Check(testkit.Rows("{}", "true"))
}

func TestSortJSONWarning(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table t(i INT, j JSON)")
tk.MustExec("insert into t values (0, json_array(5)), (1, json_array(1,2)), (2, json_array(1,2,3))")
tk.MustQuery("select i from t order by j;").Check(testkit.Rows("1", "2", "0"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1235 This version of TiDB doesn't yet support sorting of non-scalar JSON values"))

tk.MustQuery("select i,percent_rank() over (order by j) from t").Check(testkit.Rows("1 0", "2 0.5", "0 1"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1235 This version of TiDB doesn't yet support sorting of non-scalar JSON values", "Warning 1235 This version of TiDB doesn't yet support sorting of non-scalar JSON values"))
}
4 changes: 3 additions & 1 deletion planner/core/exhaust_physical_plans.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,7 @@ var symmetricOp = map[string]string{
// ColWithCmpFuncManager is used in index join to handle the column with compare functions(>=, >, <, <=).
// It stores the compare functions and build ranges in execution phase.
type ColWithCmpFuncManager struct {
ctx sessionctx.Context
TargetCol *expression.Column
colLength int
OpType []string
Expand All @@ -1188,7 +1189,7 @@ func (cwc *ColWithCmpFuncManager) appendNewExpr(opName string, arg expression.Ex
if cwc.affectedColSchema.Contains(col) {
continue
}
cwc.compareFuncs = append(cwc.compareFuncs, chunk.GetCompareFunc(col.RetType))
cwc.compareFuncs = append(cwc.compareFuncs, chunk.GetCompareFunc(col.RetType, cwc.ctx.GetSessionVars().StmtCtx.AppendWarning))
cwc.affectedColSchema.Append(col)
}
}
Expand Down Expand Up @@ -1450,6 +1451,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
}
lastPossibleCol := path.IdxCols[lastColPos]
lastColManager := &ColWithCmpFuncManager{
ctx: ijHelper.join.ctx,
TargetCol: lastPossibleCol,
colLength: path.IdxColLens[lastColPos],
affectedColSchema: expression.NewSchema(),
Expand Down
28 changes: 20 additions & 8 deletions util/chunk/compare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@ import (
"github.com/pingcap/tidb/parser/mysql"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/types/json"
"github.com/pingcap/tidb/util/dbterror"
)

var errNotSupportedYet = dbterror.ClassUtil.NewStd(mysql.ErrNotSupportedYet)

// CompareFunc is a function to compare the two values in Row, the two columns must have the same type.
type CompareFunc = func(l Row, lCol int, r Row, rCol int) int

// WarningReporter is a function to report warning to the session
type WarningReporter = func(err error)

// GetCompareFunc gets a compare function for the field type.
func GetCompareFunc(tp *types.FieldType) CompareFunc {
func GetCompareFunc(tp *types.FieldType, wp WarningReporter) CompareFunc {
switch tp.GetType() {
case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong, mysql.TypeLonglong, mysql.TypeYear:
if mysql.HasUnsignedFlag(tp.GetFlag()) {
Expand All @@ -52,7 +58,7 @@ func GetCompareFunc(tp *types.FieldType) CompareFunc {
case mysql.TypeBit:
return cmpBit
case mysql.TypeJSON:
return cmpJSON
return getCmpJSONFunction(wp)
}
return nil
}
Expand Down Expand Up @@ -160,13 +166,19 @@ func cmpBit(l Row, lCol int, r Row, rCol int) int {
return lBit.Compare(rBit)
}

func cmpJSON(l Row, lCol int, r Row, rCol int) int {
lNull, rNull := l.IsNull(lCol), r.IsNull(rCol)
if lNull || rNull {
return cmpNull(lNull, rNull)
func getCmpJSONFunction(wp WarningReporter) CompareFunc {
// TODO: deduplicate the warning in a wider range
Copy link
Member Author

Choose a reason for hiding this comment

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

We could compare against all warnings inside the sctx to check whether this warning is duplicated, but I'm not sure whether it's necessary.

notSupportedWarningReported := false
return func(l Row, lCol int, r Row, rCol int) int {
lJ, rJ := l.GetJSON(lCol), r.GetJSON(rCol)

if !notSupportedWarningReported && wp != nil && (lJ.TypeCode == json.TypeCodeArray || lJ.TypeCode == json.TypeCodeObject ||
rJ.TypeCode == json.TypeCodeArray || rJ.TypeCode == json.TypeCodeObject) {
wp(errNotSupportedYet.GenWithStack("This version of TiDB doesn't yet support sorting of non-scalar JSON values"))
notSupportedWarningReported = true
}
return json.CompareBinary(lJ, rJ)
}
lJ, rJ := l.GetJSON(lCol), r.GetJSON(rCol)
return json.CompareBinary(lJ, rJ)
}

// Compare compares the value with ad.
Expand Down