Skip to content

Commit

Permalink
*.: fix bug on column field in where | order by clause radondb#713
Browse files Browse the repository at this point in the history 
[summary]
1. add checkField() function to check the column field illegal or not if
the delete statment has field in where | order by clause.
e.g.:
mysql> delete from t where x.a=1;
ERROR 1054 (42S22): Unknown column 'x.a' in 'where clause'
mysql> delete from t where t.a=1 order by xx.a;
ERROR 1054 (42S22): Unknown column 'xx.a' in 'order clause'

2. add rewriteField() function to rewrite the column field in where | order by clause.
e.g.:
delete from t where a=1;
after rewrite maybe:
delete from test.t_0001 where test.t_0001.a=1;

[test case]
src/planner/delete_plan_test.go

[patch codecov]
src/planner/common.go  95.8%
src/planner/delete_plan.go 96.2%
  • Loading branch information
@hustjieke
hustjieke committed Jan 21, 2021
1 parent ce54316 commit 83f852b
Show file tree
Hide file tree
Showing 10 changed files with 230 additions and 60 deletions.
70 changes: 58 additions & 12 deletions intergration/radon-test/r/delete.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,29 @@ a b

INSERT INTO integrate_test.t1 VALUES (1,1);

DELETE from integrate_test.t1 where t1.a=t1.b;

select sleep(0.2);
sleep(0.2)
0

select * from integrate_test.t1;

DELETE /*test error: unkown column*/ from integrate_test.t1 where x.a=t1.b;
ERROR 1054 (42S22): Unknown column 'x.a' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=t1.b order by x.a;
ERROR 1054 (42S22): Unknown column 'x.a' in 'order clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=x.b;
ERROR 1054 (42S22): Unknown column 'x.b' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=1 order by x.x.a;
ERROR 1054 (42S22): Unknown column 'x.x.a' in 'order clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where t1.a=t1.b limit a;
ERROR 1327 (42000): Undeclared variable: a

DELETE quick from integrate_test.t1;

select * from integrate_test.t1;
Expand Down Expand Up @@ -100,18 +123,6 @@ a b c d e f g h i j
drop table integrate_test.t1;


CREATE /*test error, column not exist*/ TABLE integrate_test.t1 (
`i` int(10) NOT NULL default '0',
`i2` int(10) NOT NULL default '0',
PRIMARY KEY (`i`)
);

DELETE FROM integrate_test.t1 WHERE post='1';
ERROR 1054 (42S22): Unknown column 'post' in 'where clause'

drop table integrate_test.t1;


create /*test global*/ table integrate_test.t2 (a bigint not null auto_increment primary key, b char(32)) global;

insert into integrate_test.t2 (a,b) values (1,'apple'), (2,'apple');
Expand All @@ -130,6 +141,24 @@ ERROR 1105 (HY000): unsupported: currently.not.support.partitions.in.delete
DELETE /*test unsupport, currently not support delete with subquery*/ FROM integrate_test.t2 ORDER BY (SELECT x);
ERROR 1105 (HY000): unsupported: subqueries.in.delete

DELETE /*test error: unkown column*/ from integrate_test.t1 where x.a=t2.b;
ERROR 1054 (42S22): Unknown column 'x.a' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t2.a=t2.b order by x.a;
ERROR 1054 (42S22): Unknown column 'integrate_test.t2.a' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t2.a=x.b;
ERROR 1054 (42S22): Unknown column 'integrate_test.t2.a' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t2.a=1 order by x.x.a;
ERROR 1054 (42S22): Unknown column 'integrate_test.t2.a' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where t2.a=t2.b limit a;
ERROR 1054 (42S22): Unknown column 't2.a' in 'where clause'

DELETE quick from integrate_test.t1;
ERROR 1146 (42S02): Table 't1' doesn't exist

drop table integrate_test.t2;


Expand Down Expand Up @@ -168,6 +197,23 @@ SELECT * FROM integrate_test.t1;
a
0

DELETE /*test error: unkown column*/ from integrate_test.t1 where x.a=t1.b;
ERROR 1054 (42S22): Unknown column 'x.a' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=t1.b order by x.a;
ERROR 1054 (42S22): Unknown column 'x.a' in 'order clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=x.b;
ERROR 1054 (42S22): Unknown column 'x.b' in 'where clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=1 order by x.x.a;
ERROR 1054 (42S22): Unknown column 'x.x.a' in 'order clause'

DELETE /*test error: unkown column*/ from integrate_test.t1 where t1.a=t1.b limit a;
ERROR 1327 (42000): Undeclared variable: a

DELETE quick from integrate_test.t1;

DROP TABLE integrate_test.t1;


Expand Down
2 changes: 1 addition & 1 deletion intergration/radon-test/r/desc_explain.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ EXPLAIN: {
"RawQuery": "explain delete from integrate_test.t where c1=2",
"Partitions": [
{
"Query": "delete from integrate_test.t_0061 where c1 = 2",
"Query": "delete from integrate_test.t_0061 where integrate_test.t_0061.c1 = 2",
"Backend": "backend2",
"Range": "[3904-3968)"
}
Expand Down
8 changes: 4 additions & 4 deletions intergration/radon-test/r/replace.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,19 @@ c1 c2
2 4
3 5

replace into integrate_test.t_list values (1,4), (7,9), (2,8);
replace into integrate_test.t_list values (5,4), (7,9), (8,8);

replace /*Column count doesn't match value count*/ into integrate_test.t_list values (1,4,8), (7,9), (2,8,0);
replace /*Column count doesn't match value count*/ into integrate_test.t_list values (1,4,8), (2,8,0);
ERROR 1136 (21S01): Column count doesn't match value count at row 1

select * from integrate_test.t_list order by integrate_test.t_list.c1 asc;
c1 c2
1 4
1 2
2 8
2 4
3 5
5 4
7 9
8 8

drop table integrate_test.t_list;

Expand Down
28 changes: 20 additions & 8 deletions intergration/radon-test/t/delete.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ DELETE from integrate_test.t1 where a=1 limit 1;
DELETE LOW_PRIORITY from integrate_test.t1 where a=2;
select * from integrate_test.t1;
INSERT INTO integrate_test.t1 VALUES (1,1);
DELETE from integrate_test.t1 where t1.a=t1.b;
select sleep(0.2);
select * from integrate_test.t1;
DELETE /*test error: unkown column*/ from integrate_test.t1 where x.a=t1.b;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=t1.b order by x.a;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=x.b;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=1 order by x.x.a;
DELETE /*test error: unkown column*/ from integrate_test.t1 where t1.a=t1.b limit a;
DELETE quick from integrate_test.t1;
select * from integrate_test.t1;
drop table integrate_test.t1;
Expand Down Expand Up @@ -46,20 +54,18 @@ delete from integrate_test.t1 where a=27;
select * from integrate_test.t1 order by a;
drop table integrate_test.t1;

CREATE /*test error, column not exist*/ TABLE integrate_test.t1 (
`i` int(10) NOT NULL default '0',
`i2` int(10) NOT NULL default '0',
PRIMARY KEY (`i`)
);
DELETE FROM integrate_test.t1 WHERE post='1';
drop table integrate_test.t1;

create /*test global*/ table integrate_test.t2 (a bigint not null auto_increment primary key, b char(32)) global;
insert into integrate_test.t2 (a,b) values (1,'apple'), (2,'apple');
select * from integrate_test.t2;
delete /*test unsupport, currently not support delete multitables*/ integrate_test.t2 from integrate_test.t2, integrate_test.t2 as t1 where integrate_test.t2.b = integrate_test.t1.b and integrate_test.t2.a > integrate_test.t1.a;
delete /*test unsupport, currently not support delete with partition*/ from integrate_test.t2 partition (p0) where a = 1;
DELETE /*test unsupport, currently not support delete with subquery*/ FROM integrate_test.t2 ORDER BY (SELECT x);
DELETE /*test error: unkown column*/ from integrate_test.t1 where x.a=t2.b;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t2.a=t2.b order by x.a;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t2.a=x.b;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t2.a=1 order by x.x.a;
DELETE /*test error: unkown column*/ from integrate_test.t1 where t2.a=t2.b limit a;
DELETE quick from integrate_test.t1;
drop table integrate_test.t2;

CREATE TABLE integrate_test.t1(a INTEGER PRIMARY KEY) single;
Expand All @@ -77,6 +83,12 @@ DELETE FROM integrate_test.t1 WHERE a > 0 ORDER BY a;
INSERT INTO integrate_test.t1 VALUES (0),(1),(2);
DELETE FROM integrate_test.t1 WHERE a > 0 ORDER BY a LIMIT 1;
SELECT * FROM integrate_test.t1;
DELETE /*test error: unkown column*/ from integrate_test.t1 where x.a=t1.b;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=t1.b order by x.a;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=x.b;
DELETE /*test error: unkown column*/ from integrate_test.t1 where integrate_test.t1.a=1 order by x.x.a;
DELETE /*test error: unkown column*/ from integrate_test.t1 where t1.a=t1.b limit a;
DELETE quick from integrate_test.t1;
DROP TABLE integrate_test.t1;

drop database integrate_test;
4 changes: 2 additions & 2 deletions intergration/radon-test/t/replace.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ create /*test partition list*/ table integrate_test.t_list(c1 int, c2 int) ENGIN
insert into integrate_test.t_list values (1,2), (3,5), (2,4);
insert /*test error: has no parition*/ into integrate_test.t_list values (10,2), (13,5);
select * from integrate_test.t_list order by integrate_test.t_list.c1 asc;
replace into integrate_test.t_list values (1,4), (7,9), (2,8);
replace /*Column count doesn't match value count*/ into integrate_test.t_list values (1,4,8), (7,9), (2,8,0);
replace into integrate_test.t_list values (5,4), (7,9), (8,8);
replace /*Column count doesn't match value count*/ into integrate_test.t_list values (1,4,8), (2,8,0);
select * from integrate_test.t_list order by integrate_test.t_list.c1 asc;
drop table integrate_test.t_list;

Expand Down
89 changes: 89 additions & 0 deletions src/planner/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ package planner

import (
"errors"
"fmt"

"github.com/xelabs/go-mysqlstack/sqldb"
"github.com/xelabs/go-mysqlstack/sqlparser"
"github.com/xelabs/go-mysqlstack/xlog"
)

func hasSubquery(node sqlparser.SQLNode) bool {
Expand All @@ -38,3 +41,89 @@ func isUpdateShardKey(exprs sqlparser.UpdateExprs, shardkey string) bool {
}
return false
}

// checkField is used to check if the field's Qualifier is illegal or not in where or order by clause.
func checkField(database, table string, node sqlparser.SQLNode, log *xlog.Log) error {
switch node.(type) {
case *sqlparser.Delete:
// Here we not should do check on limit clause, leave it to the backend mysql.
nodePtr := node.(*sqlparser.Delete)
if err := checkFieldImpl(database, table, nodePtr.Where, "where clause"); err != nil {
return err
}
if err := checkFieldImpl(database, table, nodePtr.OrderBy, "order clause"); err != nil {
return err
}
default:
log.Warning("currently.check.field.only.support.delete.statement.")
}
return nil
}

// checkFieldImpl is an implementation of checkField() function.
func checkFieldImpl(database, table string, node sqlparser.SQLNode, suffix string) error {
err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
switch node.(type) {
case *sqlparser.ColName:
nodePtr := node.(*sqlparser.ColName)
colDB := nodePtr.Qualifier.Qualifier.String()
colTbl := nodePtr.Qualifier.Name.String()
colName := nodePtr.Name.String()

if colDB != "" && colTbl != "" {
// case 1: where/order by db.t.a
if colDB != database || colTbl != table {
badField := fmt.Sprintf("%s.%s.%s", colDB, colTbl, colName)
return false, sqldb.NewSQLError(sqldb.ER_BAD_FIELD_ERROR, badField, suffix)
}
} else if colDB == "" && colTbl != "" {
// case 2: where/order by t.a
if colTbl != table {
badField := fmt.Sprintf("%s.%s", colTbl, colName)
return false, sqldb.NewSQLError(sqldb.ER_BAD_FIELD_ERROR, badField, suffix)
}
}
// case 3: where/order by a, return
// Currently if column a is not in table t, the err msg output is different with mysql.
// e.g.: DELETE FROM integrate_test.t WHERE post='1';
// In radondb we'll get:
// ERROR 1054 (42S22): Unknown column 'integrate_test.t_0032.post' in 'where clause'
// But in mysql we get:
// ERROR 1054 (42S22): Unknown column 'post' in 'where clause'
return true, nil
default:
// If node is not sqlparser.ColName type, return true and continue visit.
return true, nil
}
}, node)
return err
}

// rewriteField used to rewrite column field in where/order by clause.
// Currently only used by delete statement.
func rewriteField(database, partTable string, newNode sqlparser.SQLNode, log *xlog.Log) {
switch newNode.(type) {
case *sqlparser.Delete:
// Here we should not do rewrite on limit clause, leave it to the backend mysql.
nodePtr := newNode.(*sqlparser.Delete)
rewriteFieldImpl(database, partTable, nodePtr.Where, nodePtr.OrderBy)
default:
log.Warning("currently.rewrite.field.only.support.delete.statement.")
}
}

// rewriteFieldImpl is an implementation of rewriteField() function
func rewriteFieldImpl(database, partTable string, newNodes ...sqlparser.SQLNode) {
sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
switch node.(type) {
case *sqlparser.ColName:
nodePtr := node.(*sqlparser.ColName)
nodePtr.Qualifier.Name = sqlparser.NewTableIdent(partTable)
nodePtr.Qualifier.Qualifier = sqlparser.NewTableIdent(database)
return true, nil
default:
// If newNode is not sqlparser.ColName type, return true and continue visit.
return true, nil
}
}, newNodes...)
}
21 changes: 15 additions & 6 deletions src/planner/delete_plan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@ func (p *DeletePlan) analyze() error {

// Build used to build distributed querys.
func (p *DeletePlan) Build() error {
if err := p.analyze(); err != nil {
// step 1: analyze if delete has unsupported features.
var err error
if err = p.analyze(); err != nil {
return err
}

newNode := *p.node
// For single table, the len(TableRefs)=1 and the type of TableExpr must be AliasedTableExpr.
newAliseExpr := newNode.TableRefs[0].(*sqlparser.AliasedTableExpr)
Expand All @@ -97,8 +98,13 @@ func (p *DeletePlan) Build() error {
newAliseExpr.Expr = sqlparser.TableName{Name: tableID, Qualifier: databaseID}
}

// step 2: check if the column field illegal or not if delete statement has column field.
if err = checkField(databaseID.String(), tableID.String(), &newNode, p.log); err != nil {
return err
}

// step 3: get segments
var segments []router.Segment
var err error
if newNode.Where == nil {
// delete all datas, send sql to different backends, except for single table which has only one backend.
segments, err = p.router.Lookup(databaseID.String(), tableID.String(), nil, nil)
Expand All @@ -119,10 +125,13 @@ func (p *DeletePlan) Build() error {
}
}

// Rewritten the newNode to produce a new query.
// step 4: Rewritten the newNode to produce a new query.
for _, segment := range segments {
tableID = sqlparser.NewTableIdent(segment.Table)
newAliseExpr.Expr = sqlparser.TableName{Name: tableID, Qualifier: databaseID}
// rewrite column field
rewriteField(databaseID.String(), segment.Table, &newNode, p.log)
// rewrite table expr
newTableID := sqlparser.NewTableIdent(segment.Table)
newAliseExpr.Expr = sqlparser.TableName{Name: newTableID, Qualifier: databaseID}
tuple := xcontext.QueryTuple{
Query: sqlparser.String(&newNode),
Backend: segment.Backend,
Expand Down
Loading

0 comments on commit 83f852b

Please sign in to comment.