Skip to content

Commit 76a0df9

Browse files
GrahamCampbellGrahamCampbell
committed
vtgate: merge empty reference branches through rewritten copies
Signed-off-by: Graham Campbell <hello@gjcampbell.co.uk>
1 parent 2b98771 commit 76a0df9

4 files changed

Lines changed: 140 additions & 71 deletions

File tree

‎go/vt/vtgate/planbuilder/operators/route_planning.go‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -453,21 +453,13 @@ func allRealTablesHaveCopyIn(ctx *plancontext.PlanningContext, op Operator, ks *
453453
}
454454

455455
func tableHasCopyIn(ctx *plancontext.PlanningContext, vt *vindexes.BaseTable, ks *vindexes.Keyspace) bool {
456-
if vt.Keyspace == ks {
457-
return true
458-
}
459-
if _, referenced := vt.ReferencedBy[ks.Name]; referenced {
460-
return true
461-
}
462-
if vt.Source == nil {
463-
return false
464-
}
465-
if qualifier := vt.Source.Qualifier.String(); qualifier != "" {
466-
return qualifier == ks.Name
456+
for _, name := range copyCandidates(vt, ks) {
457+
src, _, _, _, _, err := ctx.VSchema.FindTableOrVindex(name)
458+
if err == nil && src != nil && src.Keyspace == ks {
459+
return true
460+
}
467461
}
468-
// an unqualified source declaration resolves through the vschema
469-
src, _, _, _, _, err := ctx.VSchema.FindTableOrVindex(vt.Source.TableName)
470-
return err == nil && src != nil && src.Keyspace == ks
462+
return false
471463
}
472464

473465
// crossKeyspaceAccountable returns the keyspace the route genuinely reads

‎go/vt/vtgate/planbuilder/operators/route_planning_test.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,11 @@ func TestCheckCrossKeyspaceJoin(t *testing.T) {
8080
return &Table{
8181
QTable: &QueryTable{Table: sqlparser.NewTableName("ref")},
8282
VTable: &vindexes.BaseTable{
83-
Name: sqlparser.NewIdentifierCS("ref"),
84-
Keyspace: ks,
85-
ReferencedBy: map[string]*vindexes.BaseTable{copyKs: {}},
83+
Name: sqlparser.NewIdentifierCS("ref"),
84+
Keyspace: ks,
85+
ReferencedBy: map[string]*vindexes.BaseTable{
86+
copyKs: {Name: sqlparser.NewIdentifierCS("refcopy")},
87+
},
8688
},
8789
}
8890
}
@@ -287,6 +289,9 @@ func TestCheckCrossKeyspaceJoin(t *testing.T) {
287289
rhs: makeRoute(ks2),
288290
vschema: &mockVSchema{
289291
preventCrossKeyspaceReads: map[string]bool{"ks1": true, "ks2": true},
292+
tables: map[string]*vindexes.BaseTable{
293+
"refcopy": {Name: sqlparser.NewIdentifierCS("refcopy"), Keyspace: ks2},
294+
},
290295
},
291296
},
292297
{
@@ -295,6 +300,9 @@ func TestCheckCrossKeyspaceJoin(t *testing.T) {
295300
rhs: noneRouteOver(ks2, sourcedTable(ks2, "ks1")),
296301
vschema: &mockVSchema{
297302
preventCrossKeyspaceReads: map[string]bool{"ks1": true, "ks2": true},
303+
tables: map[string]*vindexes.BaseTable{
304+
"src": {Name: sqlparser.NewIdentifierCS("src"), Keyspace: ks1},
305+
},
298306
},
299307
},
300308
{
@@ -326,6 +334,9 @@ func TestCheckCrossKeyspaceJoin(t *testing.T) {
326334
rhs: makeRoute(ks2),
327335
vschema: &mockVSchema{
328336
preventCrossKeyspaceReads: map[string]bool{"ks1": true, "ks2": true},
337+
tables: map[string]*vindexes.BaseTable{
338+
"refcopy": {Name: sqlparser.NewIdentifierCS("refcopy"), Keyspace: ks2},
339+
},
329340
},
330341
expectPanic: true,
331342
},

‎go/vt/vtgate/planbuilder/operators/union_merging.go‎

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,21 @@ func tryMergeNoneUnion(
232232
routing = &AnyShardRouting{keyspace: noneKeyspaces[0]}
233233
owner = noneRoute
234234
default:
235-
// the other side's routing is retained, but only when it targets the
236-
// keyspace holding the none side's tables: they exist nowhere else.
235+
// the other side's routing is retained when it targets the keyspace
236+
// holding the none side's tables — or, failing that, when every real
237+
// table of the none side has a reference copy there: the branch is
238+
// then rewritten to read those copies, so the merged query text only
239+
// names tables that exist in that keyspace.
237240
if len(noneKeyspaces) != 1 || noneKeyspaces[0] != otherRouting.Keyspace() {
238-
return nil, nil, false
241+
rewrittenNone := rewriteNoneBranchToKeyspace(ctx, noneRoute, otherRouting.Keyspace())
242+
if rewrittenNone == nil {
243+
return nil, nil, false
244+
}
245+
if noneRoute == lhsRoute {
246+
lhsRoute = rewrittenNone
247+
} else {
248+
rhsRoute = rewrittenNone
249+
}
239250
}
240251
routing = otherRouting
241252
}
@@ -244,6 +255,57 @@ func tryMergeNoneUnion(
244255
return op, exprs, true
245256
}
246257

258+
// rewriteNoneBranchToKeyspace points the none route's tables at the reference
259+
// copies living in ks, or returns nil if any table has none. A routing
260+
// degrades to none before alternates are attached, so each copy is resolved
261+
// from the vschema through the same lookup that builds alternate routes,
262+
// keeping routing rules in effect. Only leaf routes are moved, for the same
263+
// normalization reasons as tryAlternateUnionMerge.
264+
func rewriteNoneBranchToKeyspace(ctx *plancontext.PlanningContext, route *Route, ks *vindexes.Keyspace) *Route {
265+
if ks == nil {
266+
return nil
267+
}
268+
if !ctx.SemTable.DMLTargets.IsEmpty() && TableID(route).IsOverlapping(ctx.SemTable.DMLTargets) {
269+
return nil
270+
}
271+
272+
type tableSwap struct {
273+
tbl *Table
274+
altQTable *QueryTable
275+
altVTable *vindexes.BaseTable
276+
}
277+
var swaps []tableSwap
278+
resolvable := true
279+
_ = Visit(route.Source, func(op Operator) error {
280+
if _, ok := op.(*Union); ok {
281+
resolvable = false
282+
return io.EOF
283+
}
284+
tbl, ok := op.(*Table)
285+
if !ok || tbl.VTable == nil || tbl.QTable == nil {
286+
return nil
287+
}
288+
alt := resolveTableCopyIn(ctx, tbl, ks)
289+
if alt == nil {
290+
resolvable = false
291+
return io.EOF
292+
}
293+
swaps = append(swaps, tableSwap{tbl: tbl, altQTable: alt.QTable, altVTable: alt.VTable})
294+
return nil
295+
})
296+
if !resolvable || len(swaps) == 0 {
297+
return nil
298+
}
299+
300+
for _, s := range swaps {
301+
s.tbl.QTable, s.tbl.VTable = s.altQTable, s.altVTable
302+
}
303+
304+
rewritten := *route
305+
rewritten.Routing = &NoneRouting{keyspace: ks}
306+
return &rewritten
307+
}
308+
247309
// tryAlternateUnionMerge retries a declined cross-keyspace pairing by moving
248310
// one side onto the reference copies of its tables in the other side's
249311
// keyspace. The move mutates the moved side's tables in place, so all

‎go/vt/vtgate/planbuilder/testdata/union_cases.json‎

Lines changed: 55 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@
10191019
"skip_e2e": true
10201020
},
10211021
{
1022-
"comment": "a reference-degraded none routing keeps its canonical keyspace and declines a cross-keyspace merge",
1022+
"comment": "a reference-degraded none routing merges into the keyspace holding a copy of its table",
10231023
"query": "select id from ambiguous_ref_with_source where id in (null) union select id from user",
10241024
"plan": {
10251025
"Type": "Complex",
@@ -1033,41 +1033,26 @@
10331033
"ResultColumns": 1,
10341034
"Inputs": [
10351035
{
1036-
"OperatorType": "Concatenate",
1037-
"Inputs": [
1038-
{
1039-
"OperatorType": "Route",
1040-
"Variant": "None",
1041-
"Keyspace": {
1042-
"Name": "main",
1043-
"Sharded": false
1044-
},
1045-
"FieldQuery": "select dt.c0 as id, weight_string(dt.c0) from (select id from ambiguous_ref_with_source where 1 != 1) as dt(c0) where 1 != 1",
1046-
"Query": "select dt.c0 as id, weight_string(dt.c0) from (select distinct id from ambiguous_ref_with_source where id in (null)) as dt(c0)"
1047-
},
1048-
{
1049-
"OperatorType": "Route",
1050-
"Variant": "Scatter",
1051-
"Keyspace": {
1052-
"Name": "user",
1053-
"Sharded": true
1054-
},
1055-
"FieldQuery": "select dt.c0 as id, weight_string(dt.c0) from (select id from `user` where 1 != 1) as dt(c0) where 1 != 1",
1056-
"Query": "select dt.c0 as id, weight_string(dt.c0) from (select distinct id from `user`) as dt(c0)"
1057-
}
1058-
]
1036+
"OperatorType": "Route",
1037+
"Variant": "Scatter",
1038+
"Keyspace": {
1039+
"Name": "user",
1040+
"Sharded": true
1041+
},
1042+
"FieldQuery": "select id, weight_string(id) from ambiguous_ref_with_source where 1 != 1 union select id, weight_string(id) from `user` where 1 != 1",
1043+
"Query": "select id, weight_string(id) from ambiguous_ref_with_source where id in (null) union select id, weight_string(id) from `user`"
10591044
}
10601045
]
10611046
},
10621047
"TablesUsed": [
1063-
"main.ambiguous_ref_with_source",
1048+
"user.ambiguous_ref_with_source",
10641049
"user.user"
10651050
]
10661051
},
10671052
"skip_e2e": true
10681053
},
10691054
{
1070-
"comment": "differing physical reference names must not merge: the none side's table does not exist in the other keyspace",
1055+
"comment": "a reference-degraded none branch is rewritten to the copy's physical name in the target keyspace",
10711056
"query": "select id from source_of_ref where id in (null) union select id from user",
10721057
"plan": {
10731058
"Type": "Complex",
@@ -1081,37 +1066,56 @@
10811066
"ResultColumns": 1,
10821067
"Inputs": [
10831068
{
1084-
"OperatorType": "Concatenate",
1085-
"Inputs": [
1086-
{
1087-
"OperatorType": "Route",
1088-
"Variant": "None",
1089-
"Keyspace": {
1090-
"Name": "main",
1091-
"Sharded": false
1092-
},
1093-
"FieldQuery": "select dt.c0 as id, weight_string(dt.c0) from (select id from source_of_ref where 1 != 1) as dt(c0) where 1 != 1",
1094-
"Query": "select dt.c0 as id, weight_string(dt.c0) from (select distinct id from source_of_ref where id in (null)) as dt(c0)"
1095-
},
1096-
{
1097-
"OperatorType": "Route",
1098-
"Variant": "Scatter",
1099-
"Keyspace": {
1100-
"Name": "user",
1101-
"Sharded": true
1102-
},
1103-
"FieldQuery": "select dt.c0 as id, weight_string(dt.c0) from (select id from `user` where 1 != 1) as dt(c0) where 1 != 1",
1104-
"Query": "select dt.c0 as id, weight_string(dt.c0) from (select distinct id from `user`) as dt(c0)"
1105-
}
1106-
]
1069+
"OperatorType": "Route",
1070+
"Variant": "Scatter",
1071+
"Keyspace": {
1072+
"Name": "user",
1073+
"Sharded": true
1074+
},
1075+
"FieldQuery": "select id, weight_string(id) from ref_with_source as source_of_ref where 1 != 1 union select id, weight_string(id) from `user` where 1 != 1",
1076+
"Query": "select id, weight_string(id) from ref_with_source as source_of_ref where id in (null) union select id, weight_string(id) from `user`"
11071077
}
11081078
]
11091079
},
11101080
"TablesUsed": [
1111-
"main.source_of_ref",
1081+
"user.ref_with_source",
11121082
"user.user"
11131083
]
1114-
}
1084+
},
1085+
"skip_e2e": true
1086+
},
1087+
{
1088+
"comment": "the rewrite also applies when the empty reference branch is on the right",
1089+
"query": "select id from user union select id from source_of_ref where id in (null)",
1090+
"plan": {
1091+
"Type": "Complex",
1092+
"QueryType": "SELECT",
1093+
"Original": "select id from user union select id from source_of_ref where id in (null)",
1094+
"Instructions": {
1095+
"OperatorType": "Distinct",
1096+
"Collations": [
1097+
"(0:1)"
1098+
],
1099+
"ResultColumns": 1,
1100+
"Inputs": [
1101+
{
1102+
"OperatorType": "Route",
1103+
"Variant": "Scatter",
1104+
"Keyspace": {
1105+
"Name": "user",
1106+
"Sharded": true
1107+
},
1108+
"FieldQuery": "select id, weight_string(id) from `user` where 1 != 1 union select id, weight_string(id) from ref_with_source as source_of_ref where 1 != 1",
1109+
"Query": "select id, weight_string(id) from `user` union select id, weight_string(id) from ref_with_source as source_of_ref where id in (null)"
1110+
}
1111+
]
1112+
},
1113+
"TablesUsed": [
1114+
"user.ref_with_source",
1115+
"user.user"
1116+
]
1117+
},
1118+
"skip_e2e": true
11151119
},
11161120
{
11171121
"comment": "a reference-degraded none routing whose keyspace matches the other side still merges",

0 commit comments

Comments
 (0)