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.

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

*: remove FromID from expression.Column #7157

Merged
merged 6 commits into from
Jul 30, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
*: remove FromID from expression.Column
  • Loading branch information
winoros committed Jul 25, 2018
commit 3604a964c3d9ba4625478ce0c1e484dee225c73c
6 changes: 2 additions & 4 deletions expression/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ func (col *CorrelatedColumn) resolveIndices(_ *Schema) {

// Column represents a column.
type Column struct {
FromID int
ColName model.CIStr
DBName model.CIStr
OrigTblName model.CIStr
Expand All @@ -165,7 +164,7 @@ type Column struct {
// Equal implements Expression interface.
func (col *Column) Equal(_ sessionctx.Context, expr Expression) bool {
if newCol, ok := expr.(*Column); ok {
return newCol.FromID == col.FromID && newCol.Position == col.Position
return newCol.Position == col.Position
}
return false
}
Expand Down Expand Up @@ -305,9 +304,8 @@ func (col *Column) HashCode(_ *stmtctx.StatementContext) []byte {
if len(col.hashcode) != 0 {
return col.hashcode
}
col.hashcode = make([]byte, 0, 17)
col.hashcode = make([]byte, 0, 9)
col.hashcode = append(col.hashcode, columnFlag)
col.hashcode = codec.EncodeInt(col.hashcode, int64(col.FromID))
col.hashcode = codec.EncodeInt(col.hashcode, int64(col.Position))
return col.hashcode
}
Expand Down
16 changes: 7 additions & 9 deletions expression/column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import (
func (s *testEvaluatorSuite) TestColumn(c *C) {
defer testleak.AfterTest(c)()

col := &Column{RetType: types.NewFieldType(mysql.TypeLonglong), FromID: 0, Position: 0}
col := &Column{RetType: types.NewFieldType(mysql.TypeLonglong), Position: 1}

c.Assert(col.Equal(nil, col), IsTrue)
c.Assert(col.Equal(nil, &Column{FromID: 1}), IsFalse)
c.Assert(col.Equal(nil, &Column{}), IsFalse)
c.Assert(col.IsCorrelated(), IsFalse)
c.Assert(col.Equal(nil, col.Decorrelate(nil)), IsTrue)

Expand All @@ -37,8 +37,8 @@ func (s *testEvaluatorSuite) TestColumn(c *C) {

intDatum := types.NewIntDatum(1)
corCol := &CorrelatedColumn{Column: *col, Data: &intDatum}
invalidCorCol := &CorrelatedColumn{Column: Column{FromID: 1}}
schema := NewSchema(&Column{FromID: 0, Position: 0})
invalidCorCol := &CorrelatedColumn{Column: Column{}}
schema := NewSchema(&Column{Position: 1})
c.Assert(corCol.Equal(nil, corCol), IsTrue)
c.Assert(corCol.Equal(nil, invalidCorCol), IsFalse)
c.Assert(corCol.IsCorrelated(), IsTrue)
Expand Down Expand Up @@ -96,24 +96,22 @@ func (s *testEvaluatorSuite) TestColumnHashCode(c *C) {
defer testleak.AfterTest(c)()

col1 := &Column{
FromID: 1,
Position: 12,
}
c.Assert(col1.HashCode(nil), DeepEquals, []byte{0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc})
c.Assert(col1.HashCode(nil), DeepEquals, []byte{0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc})

col2 := &Column{
FromID: 11,
Position: 2,
}
c.Assert(col2.HashCode(nil), DeepEquals, []byte{0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xb, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2})
c.Assert(col2.HashCode(nil), DeepEquals, []byte{0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2})
}

func (s *testEvaluatorSuite) TestColumn2Expr(c *C) {
defer testleak.AfterTest(c)()

cols := make([]*Column, 0, 5)
for i := 0; i < 5; i++ {
cols = append(cols, &Column{FromID: i})
cols = append(cols, &Column{Position: i})
}

exprs := Column2Exprs(cols)
Expand Down
12 changes: 6 additions & 6 deletions expression/constant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ var _ = Suite(&testExpressionSuite{})

type testExpressionSuite struct{}

func newColumn(from int) *Column {
func newColumn(id int) *Column {
return &Column{
FromID: from,
ColName: model.NewCIStr(fmt.Sprint(from)),
TblName: model.NewCIStr("t"),
DBName: model.NewCIStr("test"),
RetType: types.NewFieldType(mysql.TypeLonglong),
Position: id,
ColName: model.NewCIStr(fmt.Sprint(id)),
TblName: model.NewCIStr("t"),
DBName: model.NewCIStr("test"),
RetType: types.NewFieldType(mysql.TypeLonglong),
}
}

Expand Down
35 changes: 34 additions & 1 deletion expression/distsql_builtin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
package expression

import (
"fmt"
"time"

. "github.com/pingcap/check"
"github.com/pingcap/tidb/model"
"github.com/pingcap/tidb/mysql"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/types"
Expand All @@ -27,7 +29,38 @@ import (

var _ = Suite(&testEvalSuite{})

type testEvalSuite struct{}
type testEvalSuite struct {
colID int
}

func (s *testEvalSuite) SetUpSuite(c *C) {
s.colID = 0
}

func (s *testEvalSuite) allocColID() int {
s.colID++
return s.colID
}

// generateSchema will generate a schema for test. Used only in this file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, it is only used in schema_test.go.

func (s *testEvalSuite) generateSchema(colCount int, dbName, tblName string) *Schema {
cols := make([]*Column, 0, colCount)
for i := 0; i < colCount; i++ {
cols = append(cols, &Column{
Position: s.allocColID(),
DBName: model.NewCIStr(dbName),
TblName: model.NewCIStr(tblName),
ColName: model.NewCIStr(fmt.Sprintf("C%v", i)),
})
if i >= 2 {
cols[i].DBName = model.NewCIStr("")
}
if i >= 3 {
cols[i].TblName = model.NewCIStr("")
}
}
return NewSchema(cols...)
}

// TestEval test expr.Eval().
// TODO: add more tests.
Expand Down
1 change: 1 addition & 0 deletions expression/evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func (s *testEvaluatorSuite) TearDownSuite(c *C) {
}

func (s *testEvaluatorSuite) SetUpTest(c *C) {
s.ctx.GetSessionVars().PlanColumnID = 0
testleak.BeforeTest()
}

Expand Down
14 changes: 7 additions & 7 deletions expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,13 @@ func EvaluateExprWithNull(ctx sessionctx.Context, schema *Schema, expr Expressio
}

// TableInfo2Schema converts table info to schema with empty DBName.
func TableInfo2Schema(tbl *model.TableInfo) *Schema {
return TableInfo2SchemaWithDBName(model.CIStr{}, tbl)
func TableInfo2Schema(ctx sessionctx.Context, tbl *model.TableInfo) *Schema {
return TableInfo2SchemaWithDBName(ctx, model.CIStr{}, tbl)
}

// TableInfo2SchemaWithDBName converts table info to schema.
func TableInfo2SchemaWithDBName(dbName model.CIStr, tbl *model.TableInfo) *Schema {
cols := ColumnInfos2ColumnsWithDBName(dbName, tbl.Name, tbl.Columns)
func TableInfo2SchemaWithDBName(ctx sessionctx.Context, dbName model.CIStr, tbl *model.TableInfo) *Schema {
cols := ColumnInfos2ColumnsWithDBName(ctx, dbName, tbl.Name, tbl.Columns)
keys := make([]KeyInfo, 0, len(tbl.Indices)+1)
for _, idx := range tbl.Indices {
if !idx.Unique || idx.State != model.StatePublic {
Expand Down Expand Up @@ -300,9 +300,9 @@ func TableInfo2SchemaWithDBName(dbName model.CIStr, tbl *model.TableInfo) *Schem
}

// ColumnInfos2ColumnsWithDBName converts a slice of ColumnInfo to a slice of Column.
func ColumnInfos2ColumnsWithDBName(dbName, tblName model.CIStr, colInfos []*model.ColumnInfo) []*Column {
func ColumnInfos2ColumnsWithDBName(ctx sessionctx.Context, dbName, tblName model.CIStr, colInfos []*model.ColumnInfo) []*Column {
columns := make([]*Column, 0, len(colInfos))
for i, col := range colInfos {
for _, col := range colInfos {
if col.State != model.StatePublic {
continue
}
Expand All @@ -311,7 +311,7 @@ func ColumnInfos2ColumnsWithDBName(dbName, tblName model.CIStr, colInfos []*mode
TblName: tblName,
DBName: dbName,
RetType: &col.FieldType,
Position: i,
Position: ctx.GetSessionVars().AllocPlanColumnID(),
Index: col.Offset,
}
columns = append(columns, newCol)
Expand Down
13 changes: 8 additions & 5 deletions expression/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"time"

. "github.com/pingcap/check"
"github.com/pingcap/tidb/ast"
"github.com/pingcap/tidb/model"
"github.com/pingcap/tidb/mysql"
"github.com/pingcap/tidb/sessionctx/stmtctx"
Expand All @@ -37,18 +38,21 @@ func (s *testEvaluatorSuite) TestNewValuesFunc(c *C) {
func (s *testEvaluatorSuite) TestEvaluateExprWithNull(c *C) {
defer testleak.AfterTest(c)()
tblInfo := newTestTableBuilder("").add("col0", mysql.TypeLonglong).add("col1", mysql.TypeLonglong).build()
ifnullOuter, err := ParseSimpleExpr(s.ctx, "ifnull(col0, ifnull(col1, 1))", tblInfo)
c.Assert(err, IsNil)
schema := tableInfoToSchemaForTest(tblInfo)
col0 := schema.Columns[0]
col1 := schema.Columns[1]
schema.Columns = schema.Columns[:1]
innerIfNull, err := newFunctionForTest(s.ctx, ast.Ifnull, col1, One.Clone())
c.Assert(err, IsNil)
outerIfNull, err := newFunctionForTest(s.ctx, ast.Ifnull, col0, innerIfNull)
c.Assert(err, IsNil)

res := EvaluateExprWithNull(s.ctx, schema, ifnullOuter)
res := EvaluateExprWithNull(s.ctx, schema, outerIfNull)
c.Assert(res.String(), Equals, "ifnull(col1, 1)")

schema.Columns = append(schema.Columns, col1)
// ifnull(null, ifnull(null, 1))
res = EvaluateExprWithNull(s.ctx, schema, ifnullOuter)
res = EvaluateExprWithNull(s.ctx, schema, outerIfNull)
c.Assert(res.Equal(s.ctx, One), IsTrue)
}

Expand Down Expand Up @@ -124,7 +128,6 @@ func tableInfoToSchemaForTest(tableInfo *model.TableInfo) *Schema {
schema := NewSchema(make([]*Column, 0, len(columns))...)
for i, col := range columns {
schema.Append(&Column{
FromID: 1,
Position: i,
TblName: tableInfo.Name,
ColName: col.Name,
Expand Down
4 changes: 1 addition & 3 deletions expression/scalar_function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ func (s *testEvaluatorSuite) TestScalarFunction(c *C) {
defer testleak.AfterTest(c)()

a := &Column{
FromID: 0,
Position: 1,
TblName: model.NewCIStr("fei"),
ColName: model.NewCIStr("han"),
Expand All @@ -42,7 +41,7 @@ func (s *testEvaluatorSuite) TestScalarFunction(c *C) {
c.Assert(res, DeepEquals, []byte{0x22, 0x6c, 0x74, 0x28, 0x66, 0x65, 0x69, 0x2e, 0x68, 0x61, 0x6e, 0x2c, 0x20, 0x31, 0x29, 0x22})
c.Assert(sf.IsCorrelated(), IsFalse)
c.Assert(sf.Decorrelate(nil).Equal(s.ctx, sf), IsTrue)
c.Assert(sf.HashCode(sc), DeepEquals, []byte{0x3, 0x4, 0x6c, 0x74, 0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x5, 0xbf, 0xf0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0})
c.Assert(sf.HashCode(sc), DeepEquals, []byte{0x3, 0x4, 0x6c, 0x74, 0x1, 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x5, 0xbf, 0xf0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0})

sf = NewValuesFunc(s.ctx, 0, types.NewFieldType(mysql.TypeLonglong))
newSf, ok := sf.Clone().(*ScalarFunction)
Expand All @@ -56,7 +55,6 @@ func (s *testEvaluatorSuite) TestScalarFunction(c *C) {
func (s *testEvaluatorSuite) TestScalarFuncs2Exprs(c *C) {
defer testleak.AfterTest(c)()
a := &Column{
FromID: 0,
Position: 1,
RetType: types.NewFieldType(mysql.TypeDouble),
}
Expand Down
12 changes: 11 additions & 1 deletion expression/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,23 @@ func (s *Schema) IsUniqueKey(col *Column) bool {
// ColumnIndex finds the index for a column.
func (s *Schema) ColumnIndex(col *Column) int {
for i, c := range s.Columns {
if c.FromID == col.FromID && c.Position == col.Position {
if c.Position == col.Position {
return i
}
}
return -1
}

// FindColumnByName finds a column by its name.
func (s *Schema) FindColumnByName(name string) *Column {
for _, col := range s.Columns {
if col.ColName.L == name {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we take the DBName and TableName into consideration?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is only used for datasource which only have one table.

return col
}
}
return nil
}

// Contains checks if the schema contains the column.
func (s *Schema) Contains(col *Column) bool {
return s.ColumnIndex(col) != -1
Expand Down
Loading