Improve quote policy #1567

Merged
lunny merged 8 commits from lunny/quote_policy2 into master 2020-03-06 07:48:36 +00:00
14 changed files with 104 additions and 67 deletions
Showing only changes of commit 162c59f5ce - Show all commits

View File

@ -22,9 +22,10 @@ steps:
commands:
- make test-sqlite
- TEST_CACHE_ENABLE=true make test-sqlite
- TEST_QUOTE_POLICY=reserved make test-sqlite
- go test ./caches/... ./contexts/... ./convert/... ./core/... ./dialects/... \
./log/... ./migrate/... ./names/... ./schemas/... ./tags/... \
./internal/json/... ./internal/statements/... ./internal/utils/... \
./log/... ./migrate/... ./names/... ./schemas/... ./tags/...
when:
event:
@ -44,6 +45,7 @@ steps:
commands:
- make test-mysql
- TEST_CACHE_ENABLE=true make test-mysql
- TEST_QUOTE_POLICY=reserved make test-mysql
when:
event:
- push
@ -62,6 +64,7 @@ steps:
commands:
- make test-mysql
- TEST_CACHE_ENABLE=true make test-mysql
- TEST_QUOTE_POLICY=reserved make test-mysql
when:
event:
- push
@ -82,6 +85,7 @@ steps:
commands:
- make test-mysql
- TEST_CACHE_ENABLE=true make test-mysql
- TEST_QUOTE_POLICY=reserved make test-mysql
when:
event:
- push
@ -102,6 +106,7 @@ steps:
commands:
- make test-mymysql
- TEST_CACHE_ENABLE=true make test-mymysql
- TEST_QUOTE_POLICY=reserved make test-mymysql
when:
event:
- push
@ -120,6 +125,7 @@ steps:
commands:
- make test-postgres
- TEST_CACHE_ENABLE=true make test-postgres
- TEST_QUOTE_POLICY=reserved make test-postgres
when:
event:
- push
@ -141,6 +147,7 @@ steps:
commands:
- make test-postgres
- TEST_CACHE_ENABLE=true make test-postgres
- TEST_QUOTE_POLICY=reserved make test-postgres
when:
event:
- push
@ -159,6 +166,7 @@ steps:
commands:
- make test-mssql
- TEST_CACHE_ENABLE=true make test-mssql
- TEST_QUOTE_POLICY=reserved make test-mssql
when:
event:
- push
@ -177,6 +185,7 @@ steps:
commands:
- make test-tidb
- TEST_CACHE_ENABLE=true make test-tidb
- TEST_QUOTE_POLICY=reserved make test-tidb
when:
event:
- push

View File

@ -39,6 +39,7 @@ TEST_TIDB_USERNAME ?= root
TEST_TIDB_PASSWORD ?=
TEST_CACHE_ENABLE ?= false
TEST_QUOTE_POLICY ?= always
.PHONY: all
all: build
@ -135,73 +136,73 @@ test-cockroach\#%: go-check
.PNONY: test-mssql
test-mssql: go-check
$(GO) test -v -race -db=mssql -cache=$(TEST_CACHE_ENABLE) \
$(GO) test -v -race -db=mssql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
-conn_str="server=$(TEST_MSSQL_HOST);user id=$(TEST_MSSQL_USERNAME);password=$(TEST_MSSQL_PASSWORD);database=$(TEST_MSSQL_DBNAME)" \
-coverprofile=mssql.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-coverprofile=mssql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PNONY: test-mssql\#%
test-mssql\#%: go-check
$(GO) test -v -race -run $* -db=mssql -cache=$(TEST_CACHE_ENABLE) \
$(GO) test -v -race -run $* -db=mssql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
-conn_str="server=$(TEST_MSSQL_HOST);user id=$(TEST_MSSQL_USERNAME);password=$(TEST_MSSQL_PASSWORD);database=$(TEST_MSSQL_DBNAME)" \
-coverprofile=mssql.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-coverprofile=mssql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PNONY: test-mymysql
test-mymysql: go-check
$(GO) test -v -race -db=mymysql -cache=$(TEST_CACHE_ENABLE) \
$(GO) test -v -race -db=mymysql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
-conn_str="tcp:$(TEST_MYSQL_HOST)*$(TEST_MYSQL_DBNAME)/$(TEST_MYSQL_USERNAME)/$(TEST_MYSQL_PASSWORD)" \
-coverprofile=mymysql.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-coverprofile=mymysql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PNONY: test-mymysql\#%
test-mymysql\#%: go-check
$(GO) test -v -race -run $* -db=mymysql -cache=$(TEST_CACHE_ENABLE) \
$(GO) test -v -race -run $* -db=mymysql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
-conn_str="tcp:$(TEST_MYSQL_HOST)*$(TEST_MYSQL_DBNAME)/$(TEST_MYSQL_USERNAME)/$(TEST_MYSQL_PASSWORD)" \
-coverprofile=mymysql.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-coverprofile=mymysql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PNONY: test-mysql
test-mysql: go-check
$(GO) test -v -race -db=mysql -cache=$(TEST_CACHE_ENABLE) \
$(GO) test -v -race -db=mysql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
-conn_str="$(TEST_MYSQL_USERNAME):$(TEST_MYSQL_PASSWORD)@tcp($(TEST_MYSQL_HOST))/$(TEST_MYSQL_DBNAME)?charset=$(TEST_MYSQL_CHARSET)" \
-coverprofile=mysql.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-coverprofile=mysql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PHONY: test-mysql\#%
test-mysql\#%: go-check
$(GO) test -v -race -run $* -db=mysql -cache=$(TEST_CACHE_ENABLE) \
$(GO) test -v -race -run $* -db=mysql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
-conn_str="$(TEST_MYSQL_USERNAME):$(TEST_MYSQL_PASSWORD)@tcp($(TEST_MYSQL_HOST))/$(TEST_MYSQL_DBNAME)?charset=$(TEST_MYSQL_CHARSET)" \
-coverprofile=mysql.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-coverprofile=mysql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PNONY: test-postgres
test-postgres: go-check
$(GO) test -v -race -db=postgres -schema='$(TEST_PGSQL_SCHEMA)' -cache=$(TEST_CACHE_ENABLE) \
-conn_str="postgres://$(TEST_PGSQL_USERNAME):$(TEST_PGSQL_PASSWORD)@$(TEST_PGSQL_HOST)/$(TEST_PGSQL_DBNAME)?sslmode=disable" \
-coverprofile=postgres.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-quote=$(TEST_QUOTE_POLICY) -coverprofile=postgres.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PHONY: test-postgres\#%
test-postgres\#%: go-check
$(GO) test -v -race -run $* -db=postgres -schema='$(TEST_PGSQL_SCHEMA)' -cache=$(TEST_CACHE_ENABLE) \
-conn_str="postgres://$(TEST_PGSQL_USERNAME):$(TEST_PGSQL_PASSWORD)@$(TEST_PGSQL_HOST)/$(TEST_PGSQL_DBNAME)?sslmode=disable" \
-coverprofile=postgres.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-quote=$(TEST_QUOTE_POLICY) -coverprofile=postgres.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PHONY: test-sqlite
test-sqlite: go-check
$(GO) test -v -race -cache=$(TEST_CACHE_ENABLE) -db=sqlite3 -conn_str="./test.db?cache=shared&mode=rwc" \
-coverprofile=sqlite.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-quote=$(TEST_QUOTE_POLICY) -coverprofile=sqlite.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PHONY: test-sqlite\#%
test-sqlite\#%: go-check
$(GO) test -v -race -run $* -cache=$(TEST_CACHE_ENABLE) -db=sqlite3 -conn_str="./test.db?cache=shared&mode=rwc" \
-coverprofile=sqlite.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-quote=$(TEST_QUOTE_POLICY) -coverprofile=sqlite.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PNONY: test-tidb
test-tidb: go-check
$(GO) test -v -race -db=mysql -cache=$(TEST_CACHE_ENABLE) -ignore_select_update=true \
-conn_str="$(TEST_TIDB_USERNAME):$(TEST_TIDB_PASSWORD)@tcp($(TEST_TIDB_HOST))/$(TEST_TIDB_DBNAME)" \
-coverprofile=tidb.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-quote=$(TEST_QUOTE_POLICY) -coverprofile=tidb.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PHONY: test-tidb\#%
test-tidb\#%: go-check
$(GO) test -v -race -run $* -db=mysql -cache=$(TEST_CACHE_ENABLE) -ignore_select_update=true \
-conn_str="$(TEST_TIDB_USERNAME):$(TEST_TIDB_PASSWORD)@tcp($(TEST_TIDB_HOST))/$(TEST_TIDB_DBNAME)" \
-coverprofile=tidb.$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
-quote=$(TEST_QUOTE_POLICY) -coverprofile=tidb.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
.PHONY: vet
vet:

View File

@ -30,15 +30,24 @@ func (s *QuoteFilter) Do(sql string) string {
buf.Grow(len(sql))
var beginSingleQuote bool
var prefix = true
for i := 0; i < len(sql); i++ {
if !beginSingleQuote && sql[i] == '`' {
if prefix {
var j = i + 1
for ; j < len(sql); j++ {
if sql[j] == '`' {
break
}
}
word := sql[i+1 : j]
isReserved := s.quoter.IsReserved(word)
if isReserved {
buf.WriteByte(s.quoter.Prefix)
} else {
}
buf.WriteString(word)
if isReserved {
buf.WriteByte(s.quoter.Suffix)
}
prefix = !prefix
i = j
} else {
if sql[i] == '\'' {
beginSingleQuote = !beginSingleQuote

View File

@ -9,7 +9,7 @@ import (
)
func TestQuoteFilter_Do(t *testing.T) {
f := QuoteFilter{schemas.Quoter{'[', ']', schemas.AlwaysReverse}}
f := QuoteFilter{schemas.Quoter{'[', ']', schemas.AlwaysReserve}}
var kases = []struct {
source string
expected string

View File

@ -205,7 +205,7 @@ var (
"PROC": true,
}
mssqlQuoter = schemas.Quoter{'[', ']', schemas.AlwaysReverse}
mssqlQuoter = schemas.Quoter{'[', ']', schemas.AlwaysReserve}
)
type mssql struct {
@ -294,11 +294,11 @@ func (db *mssql) SetQuotePolicy(quotePolicy QuotePolicy) {
switch quotePolicy {
case QuotePolicyNone:
var q = mssqlQuoter
q.IsReverse = schemas.AlwaysNoReverse
q.IsReserved = schemas.AlwaysNoReserve
db.quoter = q
case QuotePolicyReserved:
var q = mssqlQuoter
q.IsReverse = db.IsReserved
q.IsReserved = db.IsReserved
db.quoter = q
case QuotePolicyAlways:
fallthrough

View File

@ -162,7 +162,7 @@ var (
"ZEROFILL": true,
}
mysqlQuoter = schemas.Quoter{'`', '`', schemas.AlwaysReverse}
mysqlQuoter = schemas.Quoter{'`', '`', schemas.AlwaysReserve}
)
type mysql struct {
@ -461,11 +461,11 @@ func (db *mysql) SetQuotePolicy(quotePolicy QuotePolicy) {
switch quotePolicy {
case QuotePolicyNone:
var q = mysqlQuoter
q.IsReverse = schemas.AlwaysNoReverse
q.IsReserved = schemas.AlwaysNoReserve
db.quoter = q
case QuotePolicyReserved:
var q = mysqlQuoter
q.IsReverse = db.IsReserved
q.IsReserved = db.IsReserved
db.quoter = q
case QuotePolicyAlways:
fallthrough

View File

@ -499,7 +499,7 @@ var (
"ZONE": true,
}
oracleQuoter = schemas.Quoter{'[', ']', schemas.AlwaysReverse}
oracleQuoter = schemas.Quoter{'[', ']', schemas.AlwaysReserve}
)
type oracle struct {
@ -623,11 +623,11 @@ func (db *oracle) SetQuotePolicy(quotePolicy QuotePolicy) {
switch quotePolicy {
case QuotePolicyNone:
var q = oracleQuoter
q.IsReverse = schemas.AlwaysNoReverse
q.IsReserved = schemas.AlwaysNoReserve
db.quoter = q
case QuotePolicyReserved:
var q = oracleQuoter
q.IsReverse = db.IsReserved
q.IsReserved = db.IsReserved
db.quoter = q
case QuotePolicyAlways:
fallthrough

View File

@ -767,7 +767,7 @@ var (
"ZONE": true,
}
postgresQuoter = schemas.Quoter{'"', '"', schemas.AlwaysReverse}
postgresQuoter = schemas.Quoter{'"', '"', schemas.AlwaysReserve}
)
const postgresPublicSchema = "public"
@ -792,11 +792,11 @@ func (db *postgres) SetQuotePolicy(quotePolicy QuotePolicy) {
switch quotePolicy {
case QuotePolicyNone:
var q = postgresQuoter
q.IsReverse = schemas.AlwaysNoReverse
q.IsReserved = schemas.AlwaysNoReserve
db.quoter = q
case QuotePolicyReserved:
var q = postgresQuoter
q.IsReverse = db.IsReserved
q.IsReserved = db.IsReserved
db.quoter = q
case QuotePolicyAlways:
fallthrough

View File

@ -144,7 +144,7 @@ var (
"WITHOUT": true,
}
sqlite3Quoter = schemas.Quoter{'`', '`', schemas.AlwaysReverse}
sqlite3Quoter = schemas.Quoter{'`', '`', schemas.AlwaysReserve}
)
type sqlite3 struct {
@ -160,11 +160,11 @@ func (db *sqlite3) SetQuotePolicy(quotePolicy QuotePolicy) {
switch quotePolicy {
case QuotePolicyNone:
var q = sqlite3Quoter
q.IsReverse = schemas.AlwaysNoReverse
q.IsReserved = schemas.AlwaysNoReserve
db.quoter = q
case QuotePolicyReserved:
var q = sqlite3Quoter
q.IsReverse = db.IsReserved
q.IsReserved = db.IsReserved
db.quoter = q
case QuotePolicyAlways:
fallthrough
@ -266,18 +266,24 @@ func (db *sqlite3) ForUpdateSQL(query string) string {
}
func (db *sqlite3) IsColumnExist(ctx context.Context, tableName, colName string) (bool, error) {
args := []interface{}{tableName}
query := "SELECT name FROM sqlite_master WHERE type='table' and name = ? and ((sql like '%`" + colName + "`%') or (sql like '%[" + colName + "]%'))"
rows, err := db.DB().QueryContext(ctx, query, args...)
query := "SELECT * FROM " + tableName + " LIMIT 0"
rows, err := db.DB().QueryContext(ctx, query)
if err != nil {
return false, err
}
defer rows.Close()
if rows.Next() {
return true, nil
cols, err := rows.Columns()
if err != nil {
return false, err
}
for _, col := range cols {
if strings.EqualFold(col, colName) {
return true, nil
}
}
return false, nil
}

View File

@ -9,6 +9,7 @@ import (
"time"
"xorm.io/xorm/caches"
"xorm.io/xorm/dialects"
"xorm.io/xorm/log"
"xorm.io/xorm/names"
)
@ -180,6 +181,13 @@ func (eg *EngineGroup) SetPolicy(policy GroupPolicy) *EngineGroup {
return eg
}
func (eg *EngineGroup) SetQuotePolicy(quotePolicy dialects.QuotePolicy) {
eg.Engine.SetQuotePolicy(quotePolicy)
for i := 0; i < len(eg.slaves); i++ {
eg.slaves[i].SetQuotePolicy(quotePolicy)
}
}
// SetTableMapper set the table name mapping rule
func (eg *EngineGroup) SetTableMapper(mapper names.Mapper) {
eg.Engine.SetTableMapper(mapper)

View File

@ -104,6 +104,7 @@ type EngineInterface interface {
SetMapper(names.Mapper)
SetMaxOpenConns(int)
SetMaxIdleConns(int)
SetQuotePolicy(dialects.QuotePolicy)
SetSchema(string)
SetTableMapper(names.Mapper)
SetTZDatabase(tz *time.Location)

View File

@ -10,23 +10,23 @@ import (
// Quoter represents a quoter to the SQL table name and column name
type Quoter struct {
Prefix byte
Suffix byte
IsReverse func(string) bool
Prefix byte
Suffix byte
IsReserved func(string) bool
}
var (
// AlwaysFalseReverse always think it's not a reverse word
AlwaysNoReverse = func(string) bool { return false }
AlwaysNoReserve = func(string) bool { return false }
// AlwaysReverse always reverse the word
AlwaysReverse = func(string) bool { return true }
AlwaysReserve = func(string) bool { return true }
// CommanQuoteMark represnets the common quote mark
CommanQuoteMark byte = '`'
// CommonQuoter represetns a common quoter
CommonQuoter = Quoter{CommanQuoteMark, CommanQuoteMark, AlwaysReverse}
CommonQuoter = Quoter{CommanQuoteMark, CommanQuoteMark, AlwaysReserve}
)
func (q Quoter) IsEmpty() bool {
@ -141,8 +141,8 @@ func (q Quoter) quoteWordTo(buf *strings.Builder, word string) error {
return err
}
isReverse := q.IsReverse(realWord)
if isReverse {
isReserved := q.IsReserved(realWord)
if isReserved {
if err := buf.WriteByte(q.Prefix); err != nil {
return err
}
@ -150,7 +150,7 @@ func (q Quoter) quoteWordTo(buf *strings.Builder, word string) error {
if _, err := buf.WriteString(realWord); err != nil {
return err
}
if isReverse {
if isReserved {
return buf.WriteByte(q.Suffix)
}

View File

@ -22,7 +22,7 @@ type NullType struct {
Age sql.NullInt64
Height sql.NullFloat64
IsMan sql.NullBool `xorm:"null"`
CustomStruct CustomStruct `xorm:"valchar(64) null"`
CustomStruct CustomStruct `xorm:"varchar(64) null"`
}
type CustomStruct struct {
@ -58,14 +58,12 @@ func (m CustomStruct) Value() (driver.Value, error) {
func TestCreateNullStructTable(t *testing.T) {
assert.NoError(t, prepareEngine())
err := testEngine.CreateTables(new(NullType))
assert.NoError(t, err)
}
func TestDropNullStructTable(t *testing.T) {
assert.NoError(t, prepareEngine())
err := testEngine.DropTables(new(NullType))
assert.NoError(t, err)
}
@ -78,7 +76,7 @@ func TestNullStructInsert(t *testing.T) {
item := new(NullType)
_, err := testEngine.Insert(item)
assert.NoError(t, err)
assert.EqualValues(t, item.Id, 1)
assert.EqualValues(t, 1, item.Id)
}
if true {
@ -90,12 +88,11 @@ func TestNullStructInsert(t *testing.T) {
}
_, err := testEngine.Insert(&item)
assert.NoError(t, err)
assert.EqualValues(t, item.Id, 2)
assert.EqualValues(t, 2, item.Id)
}
if true {
items := []NullType{}
for i := 0; i < 5; i++ {
item := NullType{
Name: sql.NullString{String: "haolei_" + fmt.Sprint(i+1), Valid: true},
@ -152,7 +149,7 @@ func TestNullStructUpdate(t *testing.T) {
affected, err := testEngine.ID(2).Cols("age", "height", "is_man").Update(item)
assert.NoError(t, err)
assert.EqualValues(t, affected, 1)
assert.EqualValues(t, 1, affected)
}
if true { // 测试In update
@ -160,7 +157,7 @@ func TestNullStructUpdate(t *testing.T) {
item.Age = sql.NullInt64{Int64: 23, Valid: true}
affected, err := testEngine.In("id", 3, 4).Cols("age", "height", "is_man").Update(item)
assert.NoError(t, err)
assert.EqualValues(t, affected, 2)
assert.EqualValues(t, 2, affected)
}
if true { // 测试where
@ -183,9 +180,7 @@ func TestNullStructUpdate(t *testing.T) {
_, err := testEngine.AllCols().ID(6).Update(item)
assert.NoError(t, err)
fmt.Println(item)
}
}
func TestNullStructFind(t *testing.T) {
@ -274,9 +269,8 @@ func TestNullStructCount(t *testing.T) {
if true {
item := new(NullType)
total, err := testEngine.Where("age IS NOT NULL").Count(item)
_, err := testEngine.Where("age IS NOT NULL").Count(item)
assert.NoError(t, err)
fmt.Println(total)
}
}
@ -292,7 +286,6 @@ func TestNullStructRows(t *testing.T) {
for rows.Next() {
err = rows.Scan(item)
assert.NoError(t, err)
fmt.Println(item)
}
}

View File

@ -18,6 +18,7 @@ import (
_ "github.com/mattn/go-sqlite3"
_ "github.com/ziutek/mymysql/godrv"
"xorm.io/xorm/caches"
"xorm.io/xorm/dialects"
"xorm.io/xorm/log"
"xorm.io/xorm/names"
"xorm.io/xorm/schemas"
@ -38,6 +39,7 @@ var (
schema = flag.String("schema", "", "specify the schema")
ignoreSelectUpdate = flag.Bool("ignore_select_update", false, "ignore select update if implementation difference, only for tidb")
ingoreUpdateLimit = flag.Bool("ignore_update_limit", false, "ignore update limit if implementation difference, only for cockroach")
quotePolicyStr = flag.String("quote", "always", "quote could be always, none, reversed")
tableMapper names.Mapper
colMapper names.Mapper
)
@ -131,6 +133,14 @@ func createEngine(dbType, connStr string) error {
testEngine.SetMapper(names.LintGonicMapper)
}
}
if *quotePolicyStr == "none" {
testEngine.SetQuotePolicy(dialects.QuotePolicyNone)
} else if *quotePolicyStr == "reserved" {
testEngine.SetQuotePolicy(dialects.QuotePolicyReserved)
} else {
testEngine.SetQuotePolicy(dialects.QuotePolicyAlways)
}
}
tableMapper = testEngine.GetTableMapper()