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

*: move ast.NewValueExpr to standalone parser_driver package #7952

Merged
merged 3 commits into from
Oct 19, 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
17 changes: 0 additions & 17 deletions ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,6 @@ type ExprNode interface {
SetType(tp *types.FieldType)
// GetType gets the evaluation type of the expression.
GetType() *types.FieldType
// SetValue sets value to the expression.
SetValue(val interface{})
// GetValue gets value of the expression.
GetValue() interface{}
// SetDatum sets datum to the expression.
SetDatum(datum types.Datum)
// GetDatum gets datum of the expression.
GetDatum() *types.Datum
// SetFlag sets flag to the expression.
// Flag indicates whether the expression contains
// parameter marker, reference, aggregate function...
Expand Down Expand Up @@ -154,15 +146,6 @@ type RecordSet interface {
Close() error
}

// RowToDatums converts row to datum slice.
func RowToDatums(row chunk.Row, fields []*ResultField) []types.Datum {
datums := make([]types.Datum, len(fields))
for i, f := range fields {
datums[i] = row.GetDatum(i, &f.Column.FieldType)
}
return datums
}

// ResultSetNode interface has a ResultFields property, represents a Node that returns result set.
// Implementations include SelectStmt, SubqueryExpr, TableSource, TableName and Join.
type ResultSetNode interface {
Expand Down
12 changes: 2 additions & 10 deletions ast/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,12 @@ func (dn *dmlNode) dmlStatement() {}
// Expression implementations should embed it in.
type exprNode struct {
node
types.Datum
Type types.FieldType
flag uint64
}

// SetDatum implements ExprNode interface.
func (en *exprNode) SetDatum(datum types.Datum) {
en.Datum = datum
}

// GetDatum implements ExprNode interface.
func (en *exprNode) GetDatum() *types.Datum {
return &en.Datum
}
// TexprNode is exported for parser driver.
type TexprNode = exprNode
Copy link
Member

Choose a reason for hiding this comment

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

How about s/exprNode/ExprNode/ and exporting it?

Copy link
Contributor Author

@tiancaiamao tiancaiamao Oct 18, 2018

Choose a reason for hiding this comment

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

  1. There is already a ExprNode, rename exprNode will cause conflict.
    ExprNode is an interface, and exprNode is the embed struct that implements ExprNode.

  2. Use a type alias here could make changes minimal.


// SetType implements ExprNode interface.
func (en *exprNode) SetType(tp *types.FieldType) {
Expand Down
110 changes: 15 additions & 95 deletions ast/expressions.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@ import (
"fmt"
"io"
"regexp"
"strconv"
"strings"

"github.com/pingcap/tidb/model"
"github.com/pingcap/tidb/mysql"
"github.com/pingcap/tidb/parser/opcode"
"github.com/pingcap/tidb/types"
)

var (
Expand All @@ -36,7 +33,6 @@ var (
_ ExprNode = &ExistsSubqueryExpr{}
_ ExprNode = &IsNullExpr{}
_ ExprNode = &IsTruthExpr{}
_ ExprNode = &ParamMarkerExpr{}
_ ExprNode = &ParenthesesExpr{}
_ ExprNode = &PatternInExpr{}
_ ExprNode = &PatternLikeExpr{}
Expand All @@ -45,89 +41,29 @@ var (
_ ExprNode = &RowExpr{}
_ ExprNode = &SubqueryExpr{}
_ ExprNode = &UnaryOperationExpr{}
_ ExprNode = &ValueExpr{}
_ ExprNode = &ValuesExpr{}
_ ExprNode = &VariableExpr{}

_ Node = &ColumnName{}
_ Node = &WhenClause{}
)

// ValueExpr is the simple value expression.
type ValueExpr struct {
exprNode
projectionOffset int
}

// Format the ExprNode into a Writer.
func (n *ValueExpr) Format(w io.Writer) {
var s string
switch n.Kind() {
case types.KindNull:
s = "NULL"
case types.KindInt64:
if n.Type.Flag&mysql.IsBooleanFlag != 0 {
if n.GetInt64() > 0 {
s = "TRUE"
} else {
s = "FALSE"
}
} else {
s = strconv.FormatInt(n.GetInt64(), 10)
}
case types.KindUint64:
s = strconv.FormatUint(n.GetUint64(), 10)
case types.KindFloat32:
s = strconv.FormatFloat(n.GetFloat64(), 'e', -1, 32)
case types.KindFloat64:
s = strconv.FormatFloat(n.GetFloat64(), 'e', -1, 64)
case types.KindString, types.KindBytes:
s = strconv.Quote(n.GetString())
case types.KindMysqlDecimal:
s = n.GetMysqlDecimal().String()
case types.KindBinaryLiteral:
if n.Type.Flag&mysql.UnsignedFlag != 0 {
s = fmt.Sprintf("x'%x'", n.GetBytes())
} else {
s = n.GetBinaryLiteral().ToBitLiteralString(true)
}
default:
panic("Can't format to string")
}
fmt.Fprint(w, s)
// ValueExpr define a interface for ValueExpr.
type ValueExpr interface {
ExprNode
SetValue(val interface{})
GetValue() interface{}
GetDatumString() string
GetString() string
GetProjectionOffset() int
SetProjectionOffset(offset int)
}

// NewValueExpr creates a ValueExpr with value, and sets default field type.
func NewValueExpr(value interface{}) *ValueExpr {
if ve, ok := value.(*ValueExpr); ok {
return ve
}
ve := &ValueExpr{}
ve.SetValue(value)
types.DefaultTypeForValue(value, &ve.Type)
ve.projectionOffset = -1
return ve
}
var NewValueExpr func(interface{}) ValueExpr

// SetProjectionOffset sets ValueExpr.projectionOffset for logical plan builder.
func (n *ValueExpr) SetProjectionOffset(offset int) {
n.projectionOffset = offset
}

// GetProjectionOffset returns ValueExpr.projectionOffset.
func (n *ValueExpr) GetProjectionOffset() int {
return n.projectionOffset
}

// Accept implements Node interface.
func (n *ValueExpr) Accept(v Visitor) (Node, bool) {
newNode, skipChildren := v.Enter(n)
if skipChildren {
return v.Leave(newNode)
}
n = newNode.(*ValueExpr)
return v.Leave(n)
}
// NewParamMarkerExpr creates a ParamMarkerExpr.
var NewParamMarkerExpr func(offset int) ParamMarkerExpr

// BetweenExpr is for "between and" or "not between and" expression.
type BetweenExpr struct {
Expand Down Expand Up @@ -721,25 +657,9 @@ func (n *PatternLikeExpr) Accept(v Visitor) (Node, bool) {

// ParamMarkerExpr expression holds a place for another expression.
// Used in parsing prepare statement.
type ParamMarkerExpr struct {
exprNode
Offset int
Order int
}

// Format the ExprNode into a Writer.
func (n *ParamMarkerExpr) Format(w io.Writer) {
panic("Not implemented")
}

// Accept implements Node Accept interface.
func (n *ParamMarkerExpr) Accept(v Visitor) (Node, bool) {
newNode, skipChildren := v.Enter(n)
if skipChildren {
return v.Leave(newNode)
}
n = newNode.(*ParamMarkerExpr)
return v.Leave(n)
type ParamMarkerExpr interface {
ValueExpr
SetOrder(int)
}

// ParenthesesExpr is the parentheses expression.
Expand Down
5 changes: 3 additions & 2 deletions ast/expressions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package ast_test
import (
. "github.com/pingcap/check"
. "github.com/pingcap/tidb/ast"
_ "github.com/pingcap/tidb/types/parser_driver"
)

var _ = Suite(&testExpressionsSuite{})
Expand Down Expand Up @@ -79,15 +80,15 @@ func (tc *testExpressionsSuite) TestExpresionsVisitorCover(c *C) {
{&ExistsSubqueryExpr{Sel: ce}, 1, 1},
{&IsNullExpr{Expr: ce}, 1, 1},
{&IsTruthExpr{Expr: ce}, 1, 1},
{&ParamMarkerExpr{}, 0, 0},
{NewParamMarkerExpr(0), 0, 0},
{&ParenthesesExpr{Expr: ce}, 1, 1},
{&PatternInExpr{Expr: ce, List: []ExprNode{ce, ce, ce}, Sel: ce}, 5, 5},
{&PatternLikeExpr{Expr: ce, Pattern: ce}, 2, 2},
{&PatternRegexpExpr{Expr: ce, Pattern: ce}, 2, 2},
{&PositionExpr{}, 0, 0},
{&RowExpr{Values: []ExprNode{ce, ce}}, 2, 2},
{&UnaryOperationExpr{V: ce}, 1, 1},
{&ValueExpr{}, 0, 0},
{NewValueExpr(0), 0, 0},
{&ValuesExpr{Column: &ColumnNameExpr{Name: &ColumnName{}}}, 0, 0},
{&VariableExpr{Value: ce}, 1, 1},
}
Expand Down
6 changes: 3 additions & 3 deletions ast/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ func (f *flagSetter) Enter(in Node) (Node, bool) {
}

func (f *flagSetter) Leave(in Node) (Node, bool) {
if x, ok := in.(ParamMarkerExpr); ok {
Copy link
Member

Choose a reason for hiding this comment

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

move this to the original place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*ParamMarkerExpr is change from struct to interface, because the old struct definition depends on types.Datum

Go can't type assert on a interface:

type T interface { ... }
switch x := in.(type) {
    case T:    // compile error
}

x.SetFlag(FlagHasParamMarker)
}
switch x := in.(type) {
case *AggregateFuncExpr:
f.aggregateFunc(x)
Expand All @@ -57,8 +60,6 @@ func (f *flagSetter) Leave(in Node) (Node, bool) {
x.SetFlag(x.Expr.GetFlag())
case *IsTruthExpr:
x.SetFlag(x.Expr.GetFlag())
case *ParamMarkerExpr:
x.SetFlag(FlagHasParamMarker)
case *ParenthesesExpr:
x.SetFlag(x.Expr.GetFlag())
case *PatternInExpr:
Expand All @@ -75,7 +76,6 @@ func (f *flagSetter) Leave(in Node) (Node, bool) {
x.SetFlag(FlagHasSubquery)
case *UnaryOperationExpr:
x.SetFlag(x.V.GetFlag())
case *ValueExpr:
case *ValuesExpr:
x.SetFlag(FlagHasReference)
case *VariableExpr:
Expand Down
4 changes: 2 additions & 2 deletions ast/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,10 @@ func (n *FuncCallExpr) specialFormatArgs(w io.Writer) bool {
n.Args[0].Format(w)
fmt.Fprint(w, ", INTERVAL ")
n.Args[1].Format(w)
fmt.Fprintf(w, " %s", n.Args[2].GetDatum().GetString())
fmt.Fprintf(w, " %s", n.Args[2].(ValueExpr).GetDatumString())
return true
case TimestampAdd, TimestampDiff:
fmt.Fprintf(w, "%s, ", n.Args[0].GetDatum().GetString())
fmt.Fprintf(w, "%s, ", n.Args[0].(ValueExpr).GetDatumString())
n.Args[1].Format(w)
fmt.Fprint(w, ", ")
n.Args[2].Format(w)
Expand Down
7 changes: 4 additions & 3 deletions ast/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ type testFunctionsSuite struct {
}

func (ts *testFunctionsSuite) TestFunctionsVisitorCover(c *C) {
valueExpr := NewValueExpr(42)
stmts := []Node{
&AggregateFuncExpr{Args: []ExprNode{&ValueExpr{}}},
&FuncCallExpr{Args: []ExprNode{&ValueExpr{}}},
&FuncCastExpr{Expr: &ValueExpr{}},
&AggregateFuncExpr{Args: []ExprNode{valueExpr}},
&FuncCallExpr{Args: []ExprNode{valueExpr}},
&FuncCastExpr{Expr: valueExpr},
}

for _, stmt := range stmts {
Expand Down
4 changes: 2 additions & 2 deletions ast/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (n *DeallocateStmt) Accept(v Visitor) (Node, bool) {
// Prepared represents a prepared statement.
type Prepared struct {
Stmt StmtNode
Params []*ParamMarkerExpr
Params []ParamMarkerExpr
SchemaVersion int64
UseCache bool
}
Expand Down Expand Up @@ -321,7 +321,7 @@ type VariableAssignment struct {
// VariableAssignment should be able to store information for SetCharset/SetPWD Stmt.
// For SetCharsetStmt, Value is charset, ExtendValue is collation.
// TODO: Use SetStmt to implement set password statement.
ExtendValue *ValueExpr
ExtendValue ValueExpr
}

// Accept implements Node interface.
Expand Down
9 changes: 5 additions & 4 deletions ast/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func (visitor1) Enter(in Node) (Node, bool) {
}

func (ts *testMiscSuite) TestMiscVisitorCover(c *C) {
valueExpr := NewValueExpr(42)
Copy link
Member

Choose a reason for hiding this comment

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

what does 42 mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

42 is the "Answer to the Ultimate Question of Life, the Universe, and Everything" .

I remember @shenli asked this question before.
I like the number, through it's meaningless here.
This code improve test coverage.

stmts := []Node{
&AdminStmt{},
&AlterUserStmt{},
Expand All @@ -53,15 +54,15 @@ func (ts *testMiscSuite) TestMiscVisitorCover(c *C) {
&CreateUserStmt{},
&DeallocateStmt{},
&DoStmt{},
&ExecuteStmt{UsingVars: []ExprNode{&ValueExpr{}}},
&ExecuteStmt{UsingVars: []ExprNode{valueExpr}},
&ExplainStmt{Stmt: &ShowStmt{}},
&GrantStmt{},
&PrepareStmt{SQLVar: &VariableExpr{Value: &ValueExpr{}}},
&PrepareStmt{SQLVar: &VariableExpr{Value: valueExpr}},
&RollbackStmt{},
&SetPwdStmt{},
&SetStmt{Variables: []*VariableAssignment{
{
Value: &ValueExpr{},
Value: valueExpr,
},
}},
&UseStmt{},
Expand All @@ -72,7 +73,7 @@ func (ts *testMiscSuite) TestMiscVisitorCover(c *C) {
},
&FlushStmt{},
&PrivElem{},
&VariableAssignment{Value: &ValueExpr{}},
&VariableAssignment{Value: valueExpr},
&KillStmt{},
&DropStatsStmt{Table: &TableName{}},
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/importer/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (col *column) parseColumnOptions(ops []*ast.ColumnOption) {
case ast.ColumnOptionPrimaryKey, ast.ColumnOptionUniqKey, ast.ColumnOptionAutoIncrement:
col.table.uniqIndices[col.name] = col
case ast.ColumnOptionComment:
col.comment = op.Expr.GetDatum().GetString()
col.comment = op.Expr.(ast.ValueExpr).GetDatumString()
}
}
}
Expand Down
Loading