From 466454db0f2e2df9ee83c07f4984de2ce409987b Mon Sep 17 00:00:00 2001 From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com> Date: Wed, 30 Oct 2024 06:54:16 +0100 Subject: [PATCH 1/3] Cherry-pick 5c08da6a5db5db5eb872608aae462f3519b38388 with conflicts --- .../planbuilder/testdata/from_cases.json | 25 +++++++++++++++++ go/vt/vtgate/semantics/analyzer_test.go | 7 +++++ go/vt/vtgate/semantics/early_rewriter.go | 10 ++++++- go/vt/vtgate/semantics/early_rewriter_test.go | 27 +++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index 8cdf4630655..b2f254369ac 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -964,6 +964,31 @@ ] } }, + { + "comment": "Ambiguous column is not a problem when we can merge and push down", + "query": "select foobar from user join music on user.id = music.user_id order by foobar", + "plan": { + "QueryType": "SELECT", + "Original": "select foobar from user join music on user.id = music.user_id order by foobar", + "Instructions": { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select foobar, weight_string(foobar) from `user`, music where 1 != 1", + "OrderBy": "(0|1) ASC", + "Query": "select foobar, weight_string(foobar) from `user`, music where `user`.id = music.user_id order by foobar asc", + "ResultColumns": 1, + "Table": "`user`, music" + }, + "TablesUsed": [ + "user.music", + "user.user" + ] + } + }, { "comment": "index hints, make sure they are not stripped.", "query": "select user.col from user use index(a)", diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 5062819a88b..ed1fa5a532a 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -1259,9 +1259,16 @@ var ks3 = &vindexes.Keyspace{ func fakeSchemaInfo() *FakeSI { si := &FakeSI{ Tables: map[string]*vindexes.Table{ +<<<<<<< HEAD "t": tableT(), "t1": tableT1(), "t2": tableT2(), +======= + "t": {Name: sqlparser.NewIdentifierCS("t"), Keyspace: unsharded}, + "t3": {Name: sqlparser.NewIdentifierCS("t3"), Keyspace: unsharded}, + "t1": {Name: sqlparser.NewIdentifierCS("t1"), Columns: cols1, ColumnListAuthoritative: true, Keyspace: ks2}, + "t2": {Name: sqlparser.NewIdentifierCS("t2"), Columns: cols2, ColumnListAuthoritative: true, Keyspace: ks3}, +>>>>>>> 5c08da6a5d (Bugfix for Panic on Joined Queries with Non-Authoritative Tables in Vitess 19.0 (#17103)) }, } return si diff --git a/go/vt/vtgate/semantics/early_rewriter.go b/go/vt/vtgate/semantics/early_rewriter.go index ce47005c1d8..a32455d7585 100644 --- a/go/vt/vtgate/semantics/early_rewriter.go +++ b/go/vt/vtgate/semantics/early_rewriter.go @@ -605,7 +605,15 @@ func (r *earlyRewriter) fillInQualifiers(cursor *sqlparser.CopyOnWriteCursor) { if !found { panic("uh oh") } - tbl := r.tables.Tables[ts.TableOffset()] + offset := ts.TableOffset() + if offset < 0 { + // this is a column that is not coming from a table - it's an alias introduced in a SELECT expression + // Example: select (1+1) as foo from bar order by foo + // we don't want to add a qualifier to foo here + cursor.Replace(sqlparser.NewColName(col.Name.String())) + return + } + tbl := r.tables.Tables[offset] tblName, err := tbl.Name() if err != nil { panic(err) diff --git a/go/vt/vtgate/semantics/early_rewriter_test.go b/go/vt/vtgate/semantics/early_rewriter_test.go index 13823e5351f..9cd409c8707 100644 --- a/go/vt/vtgate/semantics/early_rewriter_test.go +++ b/go/vt/vtgate/semantics/early_rewriter_test.go @@ -735,6 +735,33 @@ func TestOrderByColumnName(t *testing.T) { } } +func TestOrderByNotAllColumnsAuthoritative(t *testing.T) { + schemaInfo := fakeSchemaInfo() + cDB := "db" + tcases := []struct { + sql string + expSQL string + deps TableSet + }{{ + sql: "select foo from t3 join t order by foo", + expSQL: "select foo from t3 join t order by foo asc", + deps: None, + }} + for _, tcase := range tcases { + t.Run(tcase.sql, func(t *testing.T) { + ast, err := sqlparser.NewTestParser().Parse(tcase.sql) + require.NoError(t, err) + selectStatement := ast.(sqlparser.SelectStatement) + semTable, err := Analyze(selectStatement, cDB, schemaInfo) + + require.NoError(t, err) + assert.Equal(t, tcase.expSQL, sqlparser.String(selectStatement)) + orderByExpr := selectStatement.GetOrderBy()[0].Expr + assert.Equal(t, tcase.deps, semTable.RecursiveDeps(orderByExpr)) + }) + } +} + func TestSemTableDependenciesAfterExpandStar(t *testing.T) { schemaInfo := &FakeSI{Tables: map[string]*vindexes.Table{ "t1": { From 8fce5372211af7b2c9cac133ff91914572330ee3 Mon Sep 17 00:00:00 2001 From: Rohit Nayak <rohit@planetscale.com> Date: Tue, 5 Nov 2024 16:31:34 +0100 Subject: [PATCH 2/3] Fix merge issues Signed-off-by: Rohit Nayak <rohit@planetscale.com> --- go/vt/vtgate/semantics/analyzer_test.go | 7 ------- go/vt/vtgate/semantics/early_rewriter_test.go | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index ed1fa5a532a..5062819a88b 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -1259,16 +1259,9 @@ var ks3 = &vindexes.Keyspace{ func fakeSchemaInfo() *FakeSI { si := &FakeSI{ Tables: map[string]*vindexes.Table{ -<<<<<<< HEAD "t": tableT(), "t1": tableT1(), "t2": tableT2(), -======= - "t": {Name: sqlparser.NewIdentifierCS("t"), Keyspace: unsharded}, - "t3": {Name: sqlparser.NewIdentifierCS("t3"), Keyspace: unsharded}, - "t1": {Name: sqlparser.NewIdentifierCS("t1"), Columns: cols1, ColumnListAuthoritative: true, Keyspace: ks2}, - "t2": {Name: sqlparser.NewIdentifierCS("t2"), Columns: cols2, ColumnListAuthoritative: true, Keyspace: ks3}, ->>>>>>> 5c08da6a5d (Bugfix for Panic on Joined Queries with Non-Authoritative Tables in Vitess 19.0 (#17103)) }, } return si diff --git a/go/vt/vtgate/semantics/early_rewriter_test.go b/go/vt/vtgate/semantics/early_rewriter_test.go index 9cd409c8707..5286b0798a4 100644 --- a/go/vt/vtgate/semantics/early_rewriter_test.go +++ b/go/vt/vtgate/semantics/early_rewriter_test.go @@ -749,7 +749,7 @@ func TestOrderByNotAllColumnsAuthoritative(t *testing.T) { }} for _, tcase := range tcases { t.Run(tcase.sql, func(t *testing.T) { - ast, err := sqlparser.NewTestParser().Parse(tcase.sql) + ast, err := sqlparser.Parse(tcase.sql) require.NoError(t, err) selectStatement := ast.(sqlparser.SelectStatement) semTable, err := Analyze(selectStatement, cDB, schemaInfo) From a78e33a2d12bcc6e4b6d1c8aa571093713ab31db Mon Sep 17 00:00:00 2001 From: Rohit Nayak <rohit@planetscale.com> Date: Tue, 5 Nov 2024 16:33:49 +0100 Subject: [PATCH 3/3] Trigger rebuild Signed-off-by: Rohit Nayak <rohit@planetscale.com>