Skip to content

Commit

Permalink
fix that JSON type having an unexpected NOT_NULL flag
Browse files Browse the repository at this point in the history
  • Loading branch information
@SunRunAway
SunRunAway committed Aug 7, 2020
1 parent 2b05ffd commit 2520d55
Show file tree
Hide file tree
Showing 8 changed files with 188 additions and 157 deletions.
2 changes: 2 additions & 0 deletions expression/builtin_time.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2030,6 +2030,8 @@ func (c *sysDateFunctionClass) getFunction(ctx sessionctx.Context, args []Expres
return nil, err
}
bf.tp.Flen, bf.tp.Decimal = 19, 0
// Illegal parameters have been filtered out in the parser, so the result is always not null.
bf.tp.Flag |= mysql.NotNullFlag

var sig builtinFunc
if len(args) == 1 {
Expand Down
31 changes: 24 additions & 7 deletions expression/constant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package expression

import (
"fmt"
"sync"

"github.com/pingcap/parser/mysql"
"github.com/pingcap/parser/terror"
Expand Down Expand Up @@ -52,7 +53,9 @@ func NewNull() *Constant {

// Constant stands for a constant value.
type Constant struct {
Value types.Datum
Value types.Datum
// once protects the changes of RetType
once sync.Once
RetType *types.FieldType
// DeferredExpr holds deferred function in PlanCache cached plan.
// it's only used to represent non-deterministic functions(see expression.DeferredFunctions)
Expand All @@ -66,6 +69,20 @@ type Constant struct {
collationInfo
}

// Clone implements Expression interface.
func (c *Constant) Clone() Expression {
con := &Constant{
Value: c.Value,
once: sync.Once{},
RetType: c.RetType,
DeferredExpr: c.DeferredExpr,
ParamMarker: c.ParamMarker,
hashcode: c.hashcode,
collationInfo: c.collationInfo,
}
return con
}

// ParamMarker indicates param provided by COM_STMT_EXECUTE.
type ParamMarker struct {
ctx sessionctx.Context
Expand Down Expand Up @@ -94,12 +111,6 @@ func (c *Constant) MarshalJSON() ([]byte, error) {
return []byte(fmt.Sprintf("\"%s\"", c)), nil
}

// Clone implements Expression interface.
func (c *Constant) Clone() Expression {
con := *c
return &con
}

// GetType implements Expression interface.
func (c *Constant) GetType() *types.FieldType {
if c.ParamMarker != nil {
Expand All @@ -110,6 +121,12 @@ func (c *Constant) GetType() *types.FieldType {
types.DefaultParamTypeForValue(dt.GetValue(), tp)
return tp
}
if !c.Value.IsNull() {
c.once.Do(func() {
c.RetType = c.RetType.Clone()
c.RetType.Flag |= mysql.NotNullFlag
})
}
return c.RetType
}

Expand Down
228 changes: 114 additions & 114 deletions expression/typeinfer_test.go
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion planner/core/physical_plans.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ type PhysicalIndexJoin struct {
IdxColLens []int
// CompareFilters stores the filters for last column if those filters need to be evaluated during execution.
// e.g. select * from t, t1 where t.a = t1.a and t.b > t1.b and t.b < t1.b+10
// If there's index(t.a, t.b). All the filters can be used to construct index range but t.b > t1.b and t.b < t1.b=10
// If there's index(t.a, t.b). All the filters can be used to construct index range but t.b > t1.b and t.b < t1.b+10
// need to be evaluated after we fetch the data of t1.
// This struct stores them and evaluate them to ranges.
CompareFilters *ColWithCmpFuncManager
Expand Down
3 changes: 0 additions & 3 deletions server/column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ func dumpFlag(tp byte, flag uint16) uint16 {
case mysql.TypeEnum:
return flag | uint16(mysql.EnumFlag)
default:
if mysql.HasBinaryFlag(uint(flag)) {
return flag | uint16(mysql.NotNullFlag)
}
return flag
}
}
Expand Down
26 changes: 19 additions & 7 deletions server/tidb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -908,15 +908,27 @@ func (ts *tidbTestSuite) TestSumAvg(c *C) {
}

func (ts *tidbTestSuite) TestNullFlag(c *C) {
// issue #9689
qctx, err := ts.tidbdrv.OpenCtx(uint64(0), 0, uint8(tmysql.DefaultCollationID), "test", nil)
c.Assert(err, IsNil)

ctx := context.Background()
rs, err := Execute(ctx, qctx, "select 1")
c.Assert(err, IsNil)
cols := rs.Columns()
c.Assert(len(cols), Equals, 1)
expectFlag := uint16(tmysql.NotNullFlag | tmysql.BinaryFlag)
c.Assert(dumpFlag(cols[0].Type, cols[0].Flag), Equals, expectFlag)
{
// issue #9689
rs, err := Execute(ctx, qctx, "select 1")
c.Assert(err, IsNil)
cols := rs.Columns()
c.Assert(len(cols), Equals, 1)
expectFlag := uint16(tmysql.NotNullFlag | tmysql.BinaryFlag)
c.Assert(dumpFlag(cols[0].Type, cols[0].Flag), Equals, expectFlag)
}

{
// issue #19025
rs, err := Execute(ctx, qctx, "select convert('{}', JSON)")
c.Assert(err, IsNil)
cols := rs.Columns()
c.Assert(len(cols), Equals, 1)
expectFlag := uint16(tmysql.BinaryFlag)
c.Assert(dumpFlag(cols[0].Type, cols[0].Flag), Equals, expectFlag)
}
}
3 changes: 3 additions & 0 deletions types/field_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ func hasVariantFieldLength(tp *FieldType) bool {

// DefaultTypeForValue returns the default FieldType for the value.
func DefaultTypeForValue(value interface{}, tp *FieldType, char string, collate string) {
if value != nil {
tp.Flag |= mysql.NotNullFlag
}
switch x := value.(type) {
case nil:
tp.Tp = mysql.TypeNull
Expand Down
50 changes: 25 additions & 25 deletions types/field_type_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,31 +170,31 @@ func (s *testFieldTypeSuite) TestDefaultTypeForValue(c *C) {
flag uint
}{
{nil, mysql.TypeNull, 0, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{1, mysql.TypeLonglong, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{0, mysql.TypeLonglong, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{432, mysql.TypeLonglong, 3, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{4321, mysql.TypeLonglong, 4, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{1234567, mysql.TypeLonglong, 7, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{12345678, mysql.TypeLonglong, 8, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{12345678901234567, mysql.TypeLonglong, 17, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{-42, mysql.TypeLonglong, 3, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{uint64(1), mysql.TypeLonglong, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{uint64(123), mysql.TypeLonglong, 3, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{uint64(1234), mysql.TypeLonglong, 4, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{uint64(1234567), mysql.TypeLonglong, 7, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{uint64(12345678), mysql.TypeLonglong, 8, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{uint64(12345678901234567), mysql.TypeLonglong, 17, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{"abc", mysql.TypeVarString, 3, UnspecifiedLength, charset.CharsetUTF8MB4, charset.CollationUTF8MB4, 0},
{1.1, mysql.TypeDouble, 3, -1, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{[]byte("abc"), mysql.TypeBlob, 3, UnspecifiedLength, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{HexLiteral{}, mysql.TypeVarString, 0, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag},
{BitLiteral{}, mysql.TypeVarString, 0, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{NewTime(ZeroCoreTime, mysql.TypeDatetime, DefaultFsp), mysql.TypeDatetime, 19, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{NewTime(FromDate(2017, 12, 12, 12, 59, 59, 0), mysql.TypeDatetime, 3), mysql.TypeDatetime, 23, 3, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{Duration{}, mysql.TypeDuration, 8, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{&MyDecimal{}, mysql.TypeNewDecimal, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{Enum{Name: "a", Value: 1}, mysql.TypeEnum, 1, UnspecifiedLength, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{Set{Name: "a", Value: 1}, mysql.TypeSet, 1, UnspecifiedLength, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag},
{1, mysql.TypeLonglong, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{0, mysql.TypeLonglong, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{432, mysql.TypeLonglong, 3, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{4321, mysql.TypeLonglong, 4, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{1234567, mysql.TypeLonglong, 7, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{12345678, mysql.TypeLonglong, 8, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{12345678901234567, mysql.TypeLonglong, 17, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{-42, mysql.TypeLonglong, 3, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{uint64(1), mysql.TypeLonglong, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{uint64(123), mysql.TypeLonglong, 3, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{uint64(1234), mysql.TypeLonglong, 4, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{uint64(1234567), mysql.TypeLonglong, 7, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{uint64(12345678), mysql.TypeLonglong, 8, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{uint64(12345678901234567), mysql.TypeLonglong, 17, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{"abc", mysql.TypeVarString, 3, UnspecifiedLength, charset.CharsetUTF8MB4, charset.CollationUTF8MB4, mysql.NotNullFlag},
{1.1, mysql.TypeDouble, 3, -1, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{[]byte("abc"), mysql.TypeBlob, 3, UnspecifiedLength, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{HexLiteral{}, mysql.TypeVarString, 0, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.UnsignedFlag | mysql.NotNullFlag},
{BitLiteral{}, mysql.TypeVarString, 0, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{NewTime(ZeroCoreTime, mysql.TypeDatetime, DefaultFsp), mysql.TypeDatetime, 19, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{NewTime(FromDate(2017, 12, 12, 12, 59, 59, 0), mysql.TypeDatetime, 3), mysql.TypeDatetime, 23, 3, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{Duration{}, mysql.TypeDuration, 8, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{&MyDecimal{}, mysql.TypeNewDecimal, 1, 0, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{Enum{Name: "a", Value: 1}, mysql.TypeEnum, 1, UnspecifiedLength, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
{Set{Name: "a", Value: 1}, mysql.TypeSet, 1, UnspecifiedLength, charset.CharsetBin, charset.CharsetBin, mysql.BinaryFlag | mysql.NotNullFlag},
}
for _, tt := range tests {
var ft FieldType
Expand Down

0 comments on commit 2520d55

Please sign in to comment.