-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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.
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
fix alter add index on virtual column bug #7575
Changes from 11 commits
0112b14
e19e34c
177a35d
0e1cd5a
7ef6521
880bd06
2c3be20
7f5b7cc
bc00f25
9645b21
9c624ff
3ac1fb2
4e44d06
de0ae54
732c952
02c24f9
c2a3ecf
a9d653b
9c36df4
2818aab
012ea29
2916122
1da045a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -270,6 +270,12 @@ func (s *testSuite) TestAdmin(c *C) { | |
tk.MustExec("ALTER TABLE t1 ADD INDEX idx3 (c4);") | ||
tk.MustExec("admin check table t1;") | ||
|
||
// For add index on virtual column | ||
tk.MustExec("drop table if exists t1;") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add more test cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @winkyao done. |
||
tk.MustExec(" create table t1 ( b json , c int as (JSON_EXTRACT(b,'$.d')));") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove extra space. |
||
tk.MustExec("insert into t1 set b='{\"d\": 100}';") | ||
tk.MustExec("alter table t1 add index idx(c);") | ||
tk.MustExec("admin check table t1;") | ||
} | ||
|
||
func (s *testSuite) fillData(tk *testkit.TestKit, table string) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,17 @@ func ParseSimpleExprWithTableInfo(ctx sessionctx.Context, exprStr string, tableI | |
return RewriteSimpleExprWithTableInfo(ctx, tableInfo, expr) | ||
} | ||
|
||
// ParseSimpleExprCastWithTableInfo parses simple expression string to Expression. | ||
// And the expr return will cast to the target type. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And the expr returns will cast to the target type. |
||
func ParseSimpleExprCastWithTableInfo(ctx sessionctx.Context, exprStr string, tableInfo *model.TableInfo, targetFt *types.FieldType) (Expression, error) { | ||
e, err := ParseSimpleExprWithTableInfo(ctx, exprStr, tableInfo) | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
e = BuildCastFunction(ctx, e, targetFt) | ||
return e, nil | ||
} | ||
|
||
// RewriteSimpleExprWithTableInfo rewrites simple ast.ExprNode to expression.Expression. | ||
func RewriteSimpleExprWithTableInfo(ctx sessionctx.Context, tbl *model.TableInfo, expr ast.ExprNode) (Expression, error) { | ||
dbName := model.NewCIStr(ctx.GetSessionVars().CurrentDB) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
// Copyright 2018 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, | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package decoder | ||
|
||
import ( | ||
"time" | ||
|
||
"github.com/juju/errors" | ||
"github.com/pingcap/tidb/expression" | ||
"github.com/pingcap/tidb/model" | ||
"github.com/pingcap/tidb/sessionctx" | ||
"github.com/pingcap/tidb/table" | ||
"github.com/pingcap/tidb/tablecodec" | ||
"github.com/pingcap/tidb/types" | ||
"github.com/pingcap/tidb/util/chunk" | ||
) | ||
|
||
// Column contains the info and generated expr of column. | ||
type Column struct { | ||
Info *model.ColumnInfo | ||
GenExpr expression.Expression | ||
} | ||
|
||
// RowDecoder decodes a byte slice into datums and eval the generated column value. | ||
type RowDecoder struct { | ||
mutRow chunk.MutRow | ||
columns map[int64]Column | ||
colTypes map[int64]*types.FieldType | ||
haveGenColumn bool | ||
} | ||
|
||
// NewRowDecoder return a new RowDecoder. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/return/returns |
||
func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) RowDecoder { | ||
colFieldMap := make(map[int64]*types.FieldType, len(decodeColMap)) | ||
haveGenCol := false | ||
for id, col := range decodeColMap { | ||
colFieldMap[id] = &col.Info.FieldType | ||
if col.GenExpr != nil { | ||
haveGenCol = true | ||
} | ||
} | ||
if !haveGenCol { | ||
return RowDecoder{ | ||
colTypes: colFieldMap, | ||
} | ||
} | ||
|
||
tps := make([]*types.FieldType, len(cols)) | ||
for _, col := range cols { | ||
tps[col.Offset] = &col.FieldType | ||
} | ||
return RowDecoder{ | ||
mutRow: chunk.MutRowFromTypes(tps), | ||
columns: decodeColMap, | ||
colTypes: colFieldMap, | ||
haveGenColumn: haveGenCol, | ||
} | ||
} | ||
|
||
// DecodeAndEvalRowWithMap decodes a byte slice into datums and eval the generated column value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/eval/evals |
||
func (rd RowDecoder) DecodeAndEvalRowWithMap(ctx sessionctx.Context, b []byte, loc *time.Location, row map[int64]types.Datum) (map[int64]types.Datum, error) { | ||
_, err := tablecodec.DecodeRowWithMap(b, rd.colTypes, loc, row) | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
if !rd.haveGenColumn { | ||
return row, nil | ||
} | ||
|
||
for id, v := range row { | ||
rd.mutRow.SetValue(rd.columns[id].Info.Offset, v.GetValue()) | ||
} | ||
for id, col := range rd.columns { | ||
if _, ok := row[id]; ok || col.GenExpr == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we remove this judgment of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zimulala done. PTAL |
||
continue | ||
} | ||
// Eval the column value | ||
val, err := col.GenExpr.Eval(rd.mutRow.ToRow()) | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
val, err = table.CastValue(ctx, val, col.Info) | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
row[id] = val | ||
} | ||
return row, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/
col.GeneratedStored == false
/!col.GeneratedStored