From 3dce816a4cb27975d5ea4f60f633a883445057b0 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 1 Aug 2024 16:47:24 +0200 Subject: [PATCH 1/4] simplify merging logic Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/operators/join_merging.go | 2 +- .../vtgate/planbuilder/operators/sharded_routing.go | 13 +++---------- .../planbuilder/operators/subquery_planning.go | 2 +- .../planbuilder/testdata/unsupported_cases.json | 4 ++-- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index 7508a2034ce..6f2af8b5ff9 100644 --- a/go/vt/vtgate/planbuilder/operators/join_merging.go +++ b/go/vt/vtgate/planbuilder/operators/join_merging.go @@ -76,7 +76,7 @@ func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPr // sharded routing is complex, so we handle it in a separate method case a == sharded && b == sharded: - return tryMergeShardedRouting(ctx, lhsRoute, rhsRoute, m, joinPredicates, false /*isSubquery*/) + return tryMergeShardedRouting(ctx, lhsRoute, rhsRoute, m, joinPredicates) default: return nil diff --git a/go/vt/vtgate/planbuilder/operators/sharded_routing.go b/go/vt/vtgate/planbuilder/operators/sharded_routing.go index 40532f729c3..066cb47d9a9 100644 --- a/go/vt/vtgate/planbuilder/operators/sharded_routing.go +++ b/go/vt/vtgate/planbuilder/operators/sharded_routing.go @@ -23,7 +23,6 @@ import ( "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/slice" "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" @@ -639,9 +638,10 @@ func tryMergeShardedRouting( routeA, routeB *Route, m merger, joinPredicates []sqlparser.Expr, - isSubquery bool, ) *Route { - sameKeyspace := routeA.Routing.Keyspace() == routeB.Routing.Keyspace() + if routeA.Routing.Keyspace() != routeB.Routing.Keyspace() { + return nil + } tblA := routeA.Routing.(*ShardedRouting) tblB := routeB.Routing.(*ShardedRouting) @@ -670,13 +670,6 @@ func tryMergeShardedRouting( return nil } - if !sameKeyspace { - if isSubquery { - panic(vterrors.VT12001("cross-shard correlated subquery")) - } - return nil - } - canMerge := canMergeOnFilters(ctx, routeA, routeB, joinPredicates) if !canMerge { return nil diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index d8427fce09f..0893afbeead 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -758,7 +758,7 @@ func mergeSubqueryInputs(ctx *plancontext.PlanningContext, in, out Operator, joi // sharded routing is complex, so we handle it in a separate method case inner == sharded && outer == sharded: - return tryMergeShardedRouting(ctx, inRoute, outRoute, m, joinPredicates, true /*isSubquery*/) + return tryMergeShardedRouting(ctx, inRoute, outRoute, m, joinPredicates) default: return nil diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json index 287e5cc992c..0e230b3e44d 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json @@ -296,8 +296,8 @@ }, { "comment": "Cross keyspace query with subquery", - "query": "select 1 from user where id in (select id from t1)", - "plan": "VT12001: unsupported: cross-shard correlated subquery" + "query": "select 1 from user where id = (select id from t1 where user.foo = t1.bar)", + "plan": "VT12001: unsupported: correlated subquery is only supported for EXISTS" }, { "comment": "multi-shard union", From f04d74082308e4e31ee4da0e35f2709e0695b663 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 5 Aug 2024 07:11:04 +0200 Subject: [PATCH 2/4] add end2end test Signed-off-by: Andres Taylor --- .../vtgate/vitess_tester/two_sharded_keyspaces/queries.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test b/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test index f625333313a..cd42fa4f680 100644 --- a/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test +++ b/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test @@ -24,3 +24,6 @@ insert into corder.corder(order_id, customer_id, sku, price) values(4, 4, 'SKU-1 insert into corder.corder(order_id, customer_id, sku, price) values(5, 5, 'SKU-1002', 30); select co.order_id, co.customer_id, co.price from corder.corder co left join customer.customer cu on co.customer_id=cu.customer_id where cu.customer_id=1; + +# This query was accidentally disallowed by https://github.com/vitessio/vitess/pull/16520 +select 1 from customer.customer where id in (select customer_id from corder.corder where price > 50); \ No newline at end of file From 15e4011802e10f02b7dfb40725bfb0d9c65eba25 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 5 Aug 2024 07:30:55 +0200 Subject: [PATCH 3/4] make sure to trigger vitess-tester workflows Signed-off-by: Andres Taylor --- .github/workflows/vitess_tester_vtgate.yml | 1 + test/templates/cluster_vitess_tester.tpl | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/vitess_tester_vtgate.yml b/.github/workflows/vitess_tester_vtgate.yml index 3aa2450ecbb..89d52fba95c 100644 --- a/.github/workflows/vitess_tester_vtgate.yml +++ b/.github/workflows/vitess_tester_vtgate.yml @@ -57,6 +57,7 @@ jobs: end_to_end: - 'go/**/*.go' - 'go/vt/sidecardb/**/*.sql' + - 'go/test/endtoend/vtgate/vitess_tester/**' - 'go/test/endtoend/onlineddl/vrepl_suite/**' - 'test.go' - 'Makefile' diff --git a/test/templates/cluster_vitess_tester.tpl b/test/templates/cluster_vitess_tester.tpl index ead64a909d2..1a479d5c558 100644 --- a/test/templates/cluster_vitess_tester.tpl +++ b/test/templates/cluster_vitess_tester.tpl @@ -55,6 +55,7 @@ jobs: end_to_end: - 'go/**/*.go' - 'go/vt/sidecardb/**/*.sql' + - 'go/test/endtoend/vtgate/vitess_tester/**' - 'go/test/endtoend/onlineddl/vrepl_suite/**' - 'test.go' - 'Makefile' From c045042cbed47bab8477886cbeb10ded9878b52d Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 5 Aug 2024 07:48:39 +0200 Subject: [PATCH 4/4] small tweaks to test and workflow Signed-off-by: Andres Taylor --- .github/workflows/vitess_tester_vtgate.yml | 1 - .../two_sharded_keyspaces/queries.test | 58 +++++++++++-------- test/templates/cluster_vitess_tester.tpl | 1 - 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/.github/workflows/vitess_tester_vtgate.yml b/.github/workflows/vitess_tester_vtgate.yml index 89d52fba95c..9e11697f778 100644 --- a/.github/workflows/vitess_tester_vtgate.yml +++ b/.github/workflows/vitess_tester_vtgate.yml @@ -58,7 +58,6 @@ jobs: - 'go/**/*.go' - 'go/vt/sidecardb/**/*.sql' - 'go/test/endtoend/vtgate/vitess_tester/**' - - 'go/test/endtoend/onlineddl/vrepl_suite/**' - 'test.go' - 'Makefile' - 'build.env' diff --git a/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test b/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test index cd42fa4f680..28c55e559c9 100644 --- a/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test +++ b/go/test/endtoend/vtgate/vitess_tester/two_sharded_keyspaces/queries.test @@ -1,29 +1,39 @@ use customer; -create table if not exists customer( - customer_id bigint not null, - email varbinary(128), - primary key(customer_id) -) ENGINE=InnoDB; -insert into customer.customer(customer_id, email) values(1, '[alice@domain.com](mailto:alice@domain.com)'); -insert into customer.customer(customer_id, email) values(2, '[bob@domain.com](mailto:bob@domain.com)'); -insert into customer.customer(customer_id, email) values(3, '[charlie@domain.com](mailto:charlie@domain.com)'); -insert into customer.customer(customer_id, email) values(4, '[dan@domain.com](mailto:dan@domain.com)'); -insert into customer.customer(customer_id, email) values(5, '[eve@domain.com](mailto:eve@domain.com)'); +create table if not exists customer +( + customer_id bigint not null, + email varbinary(128), + primary key (customer_id) +) ENGINE = InnoDB; + +insert into customer.customer(customer_id, email) +values (1, '[alice@domain.com](mailto:alice@domain.com)'), + (2, '[bob@domain.com](mailto:bob@domain.com)'), + (3, '[charlie@domain.com](mailto:charlie@domain.com)'), + (4, '[dan@domain.com](mailto:dan@domain.com)'), + (5, '[eve@domain.com](mailto:eve@domain.com)'); use corder; -create table if not exists corder( - order_id bigint not null, - customer_id bigint, - sku varbinary(128), - price bigint, - primary key(order_id) -) ENGINE=InnoDB; -insert into corder.corder(order_id, customer_id, sku, price) values(1, 1, 'SKU-1001', 100); -insert into corder.corder(order_id, customer_id, sku, price) values(2, 2, 'SKU-1002', 30); -insert into corder.corder(order_id, customer_id, sku, price) values(3, 3, 'SKU-1002', 30); -insert into corder.corder(order_id, customer_id, sku, price) values(4, 4, 'SKU-1002', 30); -insert into corder.corder(order_id, customer_id, sku, price) values(5, 5, 'SKU-1002', 30); +create table if not exists corder +( + order_id bigint not null, + customer_id bigint, + sku varbinary(128), + price bigint, + primary key (order_id) +) ENGINE = InnoDB; +insert into corder.corder(order_id, customer_id, sku, price) +values (1, 1, 'SKU-1001', 100), + (2, 2, 'SKU-1002', 30), + (3, 3, 'SKU-1002', 30), + (4, 4, 'SKU-1002', 30), + (5, 5, 'SKU-1002', 30); -select co.order_id, co.customer_id, co.price from corder.corder co left join customer.customer cu on co.customer_id=cu.customer_id where cu.customer_id=1; +select co.order_id, co.customer_id, co.price +from corder.corder co + left join customer.customer cu on co.customer_id = cu.customer_id +where cu.customer_id = 1; # This query was accidentally disallowed by https://github.com/vitessio/vitess/pull/16520 -select 1 from customer.customer where id in (select customer_id from corder.corder where price > 50); \ No newline at end of file +select 1 +from customer.customer +where customer_id in (select customer_id from corder.corder where price > 50); \ No newline at end of file diff --git a/test/templates/cluster_vitess_tester.tpl b/test/templates/cluster_vitess_tester.tpl index 1a479d5c558..541bfd5c6a0 100644 --- a/test/templates/cluster_vitess_tester.tpl +++ b/test/templates/cluster_vitess_tester.tpl @@ -56,7 +56,6 @@ jobs: - 'go/**/*.go' - 'go/vt/sidecardb/**/*.sql' - 'go/test/endtoend/vtgate/vitess_tester/**' - - 'go/test/endtoend/onlineddl/vrepl_suite/**' - 'test.go' - 'Makefile' - 'build.env'