From 630fbbfecafec94968713a2903eb033f767a806c Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Tue, 14 Aug 2018 11:53:53 -0700 Subject: [PATCH] vtgate planbuilder: verify same vindex When two parts of a query use the same unique vindex values, we also have to check that they use the same vindex. Otherwise, we end up merging queries that use different videxes but coincidentally have the same values. I've consolidated all places that perform this check and added the vindex check in that function. Signed-off-by: Sugu Sougoumarane --- data/test/vtgate/filter_cases.txt | 2 +- data/test/vtgate/schema_test.json | 2 +- data/test/vtgate/unsupported_cases.txt | 24 ++++++++++++++---------- go/vt/vtgate/planbuilder/from.go | 6 ++---- go/vt/vtgate/planbuilder/route.go | 22 ++++++++++++---------- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/data/test/vtgate/filter_cases.txt b/data/test/vtgate/filter_cases.txt index e06b4723c2a..fc56c2d397b 100644 --- a/data/test/vtgate/filter_cases.txt +++ b/data/test/vtgate/filter_cases.txt @@ -766,4 +766,4 @@ # but they refer to different things. The first reference is to the outermost query, # and the second reference is to the the innermost 'from' subquery. "select id2 from user uu where id in (select id from user where id = uu.id and user.col in (select col from (select id from user_extra where user_id = 5) uu where uu.user_id = uu.id))" -"unsupported: subquery and parent route to different shards" +"unsupported: UNION or subquery on different shards: vindex values are different" diff --git a/data/test/vtgate/schema_test.json b/data/test/vtgate/schema_test.json index 5e6ad7b1c8c..a0c8012f8c9 100644 --- a/data/test/vtgate/schema_test.json +++ b/data/test/vtgate/schema_test.json @@ -12,7 +12,7 @@ "owner": "multicolvin" }, "user_md5_index": { - "type": "hash_test" + "type": "unicode_loose_md5" }, "music_user_map": { "type": "lookup_test", diff --git a/data/test/vtgate/unsupported_cases.txt b/data/test/vtgate/unsupported_cases.txt index 897e4d128a1..d6a610a2aae 100644 --- a/data/test/vtgate/unsupported_cases.txt +++ b/data/test/vtgate/unsupported_cases.txt @@ -1,6 +1,6 @@ # Unions "select * from user union select * from user_extra" -"unsupported: UNION on multi-shard queries" +"unsupported: UNION or subquery containing multi-shard queries" # SET "set a=1" @@ -29,11 +29,11 @@ # union operations in subqueries (FROM) "select * from (select * from user union all select * from user_extra) as t" -"unsupported: UNION on multi-shard queries" +"unsupported: UNION or subquery containing multi-shard queries" # union operations in subqueries (expressions) "select * from user where id in (select * from user union select * from user_extra)" -"unsupported: UNION on multi-shard queries" +"unsupported: UNION or subquery containing multi-shard queries" # subquery with join primitive (expressions) "select * from user where id in (select user.id from user join user_extra)" @@ -77,11 +77,11 @@ # subquery does not depend on unique vindex of outer query "select id from user where id in (select user_id from user_extra where user_extra.user_id = user.col)" -"unsupported: subquery does not depend on scatter outer query" +"unsupported: UNION or subquery containing multi-shard queries" # subquery does not depend on scatter outer query "select id from user where id in (select user_id from user_extra where user_extra.user_id = 4)" -"unsupported: subquery does not depend on scatter outer query" +"unsupported: UNION or subquery containing multi-shard queries" # subquery depends on a cross-shard subquery "select id from (select user.id, user.col from user join user_extra) as t where id in (select t.col from user)" @@ -105,7 +105,7 @@ # subquery and outer query route to different shards "select id from user where id = 5 and id in (select user_id from user_extra where user_extra.user_id = 4)" -"unsupported: subquery and parent route to different shards" +"unsupported: UNION or subquery on different shards: vindex values are different" # last_insert_id for sharded keyspace "select last_insert_id() from user" @@ -433,7 +433,7 @@ # multi-shard union "(select id from user union select id from music) union select 1 from dual" -"unsupported: UNION on multi-shard queries" +"unsupported: UNION or subquery containing multi-shard queries" # multi-shard union "select 1 from music union (select id from user union all select name from unsharded)" @@ -445,15 +445,19 @@ # multi-shard union "select id from user union all select id from music" -"unsupported: UNION on multi-shard queries" +"unsupported: UNION or subquery containing multi-shard queries" + +# union with the same target shard because of vindex +"select * from music where id = 1 union select * from user where id = 1" +"unsupported: UNION or subquery on different shards: vindexes are different" # union with different target shards "select 1 from music where id = 1 union select 1 from music where id = 2" -"unsupported: UNION queries with different target shards" +"unsupported: UNION or subquery on different shards: vindex values are different" # Union all "select col1, col2 from user union all select col1, col2 from user_extra" -"unsupported: UNION on multi-shard queries" +"unsupported: UNION or subquery containing multi-shard queries" "(select user.id, user.name from user join user_extra where user_extra.extra = 'asdf') union select 'b','c' from user" "unsupported construct: SELECT of UNION is non-trivial" diff --git a/go/vt/vtgate/planbuilder/from.go b/go/vt/vtgate/planbuilder/from.go index cc0571821fe..69bf468fee8 100644 --- a/go/vt/vtgate/planbuilder/from.go +++ b/go/vt/vtgate/planbuilder/from.go @@ -276,10 +276,8 @@ func (pb *primitiveBuilder) join(rpb *primitiveBuilder, ajoin *sqlparser.JoinTab } // Both l & r routes point to the same shard. - if lRoute.ERoute.Opcode == engine.SelectEqualUnique && rRoute.ERoute.Opcode == engine.SelectEqualUnique { - if valEqual(lRoute.condition, rRoute.condition) { - return pb.mergeRoutes(rpb, ajoin) - } + if lRoute.isSameShardedRoute(rRoute) == nil { + return pb.mergeRoutes(rpb, ajoin) } } diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go index e0544368b5a..92ed6126188 100644 --- a/go/vt/vtgate/planbuilder/route.go +++ b/go/vt/vtgate/planbuilder/route.go @@ -622,14 +622,7 @@ func (rb *route) SubqueryCanMerge(pb *primitiveBuilder, inner *route) error { default: return errors.New("unsupported: scatter subquery") } - - if rb.ERoute.Opcode != engine.SelectEqualUnique { - return errors.New("unsupported: subquery does not depend on scatter outer query") - } - if !valEqual(rb.condition, inner.condition) { - return errors.New("unsupported: subquery and parent route to different shards") - } - return nil + return rb.isSameShardedRoute(inner) } // UnionCanMerge returns nil if the supplied route that represents @@ -654,12 +647,21 @@ func (rb *route) UnionCanMerge(right *route) error { } return errIntermixingUnsupported } + return rb.isSameShardedRoute(right) +} +// isSameShardedRoute returns nil if the supplied route has +// the same single shard target as the current route. If not, it +// returns an appropriate error. +func (rb *route) isSameShardedRoute(right *route) error { if rb.ERoute.Opcode != engine.SelectEqualUnique || right.ERoute.Opcode != engine.SelectEqualUnique { - return errors.New("unsupported: UNION on multi-shard queries") + return errors.New("unsupported: UNION or subquery containing multi-shard queries") + } + if rb.ERoute.Vindex != right.ERoute.Vindex { + return errors.New("unsupported: UNION or subquery on different shards: vindexes are different") } if !valEqual(rb.condition, right.condition) { - return errors.New("unsupported: UNION queries with different target shards") + return errors.New("unsupported: UNION or subquery on different shards: vindex values are different") } return nil }