From e18974d447bb5d714e7562c4383012201dc33ffd Mon Sep 17 00:00:00 2001 From: Florent Poinsard <35779988+frouioui@users.noreply.github.com> Date: Fri, 14 Apr 2023 11:22:01 +0200 Subject: [PATCH] [release-16.0] planner fix: scoping rules for JOIN ON expression inside a subquery (#12891) * planner fix: scoping rules for JOIN ON expression inside a subquery (#12881) * fix: scoping rules for on clause expression inside a subquery Signed-off-by: Harshit Gangal * find the parent scope of the statement in on clause Signed-off-by: harshit-gangal * addressed review comments Signed-off-by: harshit-gangal --------- Signed-off-by: Harshit Gangal Signed-off-by: harshit-gangal Signed-off-by: Florent Poinsard * Fix plan test expectation for v3 Signed-off-by: Florent Poinsard --------- Signed-off-by: Harshit Gangal Signed-off-by: harshit-gangal Signed-off-by: Florent Poinsard Co-authored-by: Harshit Gangal --- .../planbuilder/testdata/select_cases.json | 27 +++++++++++++++++ go/vt/vtgate/semantics/analyzer_test.go | 30 ++++++++++++++++--- go/vt/vtgate/semantics/early_rewriter_test.go | 6 ++-- go/vt/vtgate/semantics/scoper.go | 22 +++++++++++--- go/vt/vtgate/semantics/semantic_state.go | 2 +- go/vt/vtgate/semantics/table_set.go | 2 +- 6 files changed, 76 insertions(+), 13 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index 6adcf331f25..d1a69fbc7a9 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -8041,5 +8041,32 @@ "main.unsharded_a" ] } + }, + { + "comment": "subquery having join table on clause, using column reference of outer select table", + "query": "select (select 1 from user u1 join user u2 on u1.id = u2.id and u1.id = u3.id) subquery from user u3 where u3.id = 1", + "v3-plan": "VT03019: symbol u3.id not found", + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select (select 1 from user u1 join user u2 on u1.id = u2.id and u1.id = u3.id) subquery from user u3 where u3.id = 1", + "Instructions": { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select (select 1 from `user` as u1 join `user` as u2 on u1.id = u2.id and u1.id = u3.id where 1 != 1) as subquery from `user` as u3 where 1 != 1", + "Query": "select (select 1 from `user` as u1 join `user` as u2 on u1.id = u2.id and u1.id = u3.id) as subquery from `user` as u3 where u3.id = 1", + "Table": "`user`", + "Values": [ + "INT64(1)" + ], + "Vindex": "user_index" + }, + "TablesUsed": [ + "user.user" + ] + } } ] diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 96bce0b7edf..fc87946c71f 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -320,7 +320,7 @@ func TestMissingTable(t *testing.T) { func TestUnknownColumnMap2(t *testing.T) { varchar := querypb.Type_VARCHAR - int := querypb.Type_INT32 + integer := querypb.Type_INT32 authoritativeTblA := vindexes.Table{ Name: sqlparser.NewIdentifierCS("a"), @@ -346,7 +346,7 @@ func TestUnknownColumnMap2(t *testing.T) { Name: sqlparser.NewIdentifierCS("a"), Columns: []vindexes.Column{{ Name: sqlparser.NewIdentifierCI("col"), - Type: int, + Type: integer, }}, ColumnListAuthoritative: true, } @@ -354,7 +354,7 @@ func TestUnknownColumnMap2(t *testing.T) { Name: sqlparser.NewIdentifierCS("b"), Columns: []vindexes.Column{{ Name: sqlparser.NewIdentifierCI("col"), - Type: int, + Type: integer, }}, ColumnListAuthoritative: true, } @@ -391,7 +391,7 @@ func TestUnknownColumnMap2(t *testing.T) { name: "authoritative columns", schema: map[string]*vindexes.Table{"a": &authoritativeTblA, "b": &authoritativeTblBWithInt}, err: false, - typ: &int, + typ: &integer, }, { name: "authoritative columns with overlap", schema: map[string]*vindexes.Table{"a": &authoritativeTblAWithConflict, "b": &authoritativeTblB}, @@ -1434,6 +1434,28 @@ func TestSingleUnshardedKeyspace(t *testing.T) { } } +// TestScopingSubQueryJoinClause tests the scoping behavior of a subquery containing a join clause. +// The test ensures that the scoping analysis correctly identifies and handles the relationships +// between the tables involved in the join operation with the outer query. +func TestScopingSubQueryJoinClause(t *testing.T) { + query := "select (select 1 from u1 join u2 on u1.id = u2.id and u2.id = u3.id) x from u3" + + parse, err := sqlparser.Parse(query) + require.NoError(t, err) + + st, err := Analyze(parse, "user", &FakeSI{ + Tables: map[string]*vindexes.Table{ + "t": {Name: sqlparser.NewIdentifierCS("t")}, + }, + }) + require.NoError(t, err) + require.NoError(t, st.NotUnshardedErr) + + tb := st.DirectDeps(parse.(*sqlparser.Select).SelectExprs[0].(*sqlparser.AliasedExpr).Expr.(*sqlparser.Subquery).Select.(*sqlparser.Select).From[0].(*sqlparser.JoinTableExpr).Condition.On) + require.Equal(t, 3, tb.NumberOfTables()) + +} + var ks1 = &vindexes.Keyspace{ Name: "ks1", Sharded: false, diff --git a/go/vt/vtgate/semantics/early_rewriter_test.go b/go/vt/vtgate/semantics/early_rewriter_test.go index 1edd45a9c4d..324172ab72e 100644 --- a/go/vt/vtgate/semantics/early_rewriter_test.go +++ b/go/vt/vtgate/semantics/early_rewriter_test.go @@ -180,6 +180,9 @@ func TestExpandStar(t *testing.T) { require.True(t, isSelectStatement, "analyzer expects a select statement") st, err := Analyze(selectStatement, cDB, schemaInfo) if tcase.expErr == "" { + require.NoError(t, err) + require.NoError(t, st.NotUnshardedErr) + require.NoError(t, st.NotSingleRouteErr) found := 0 outer: for _, selExpr := range selectStatement.SelectExprs { @@ -204,9 +207,6 @@ func TestExpandStar(t *testing.T) { } else { require.Equal(t, tcase.colExpandedNumber, found) } - require.NoError(t, err) - require.NoError(t, st.NotUnshardedErr) - require.NoError(t, st.NotSingleRouteErr) assert.Equal(t, tcase.expSQL, sqlparser.String(selectStatement)) } else { require.EqualError(t, err, tcase.expErr) diff --git a/go/vt/vtgate/semantics/scoper.go b/go/vt/vtgate/semantics/scoper.go index adae1319e37..e61a9130b86 100644 --- a/go/vt/vtgate/semantics/scoper.go +++ b/go/vt/vtgate/semantics/scoper.go @@ -46,6 +46,7 @@ type ( tables []TableInfo isUnion bool joinUsing map[string]TableSet + stmtScope bool } ) @@ -62,11 +63,13 @@ func (s *scoper) down(cursor *sqlparser.Cursor) error { switch node := node.(type) { case *sqlparser.Update, *sqlparser.Delete: currScope := newScope(s.currentScope()) + currScope.stmtScope = true s.push(currScope) currScope.stmt = node.(sqlparser.Statement) case *sqlparser.Select: currScope := newScope(s.currentScope()) + currScope.stmtScope = true s.push(currScope) // Needed for order by with Literal to find the Expression. @@ -77,10 +80,10 @@ func (s *scoper) down(cursor *sqlparser.Cursor) error { case sqlparser.TableExpr: if isParentSelect(cursor) { // when checking the expressions used in JOIN conditions, special rules apply where the ON expression - // can only see the two tables involved in the JOIN, and no other tables. - // To create this special context, we create a special scope here that is then merged with - // the surrounding scope when we come back out from the JOIN - nScope := newScope(nil) + // can only see the two tables involved in the JOIN, and no other tables of that select statement. + // They are allowed to see the tables of the outer select query. + // To create this special context, we will find the parent scope of the select statement involved. + nScope := newScope(s.currentScope().findParentScopeOfStatement()) nScope.stmt = cursor.Parent().(*sqlparser.Select) s.push(nScope) } @@ -289,3 +292,14 @@ func (s *scope) prepareUsingMap() (result map[TableSet]map[string]TableSet) { } return } + +// findParentScopeOfStatement finds the scope that belongs to a statement. +func (s *scope) findParentScopeOfStatement() *scope { + if s.stmtScope { + return s.parent + } + if s.parent == nil { + return nil + } + return s.parent.findParentScopeOfStatement() +} diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 7d838eb286a..b3163ae7fb0 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -364,7 +364,7 @@ var _ evalengine.TranslationLookup = (*SemTable)(nil) var columnNotSupportedErr = vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "column access not supported here") // ColumnLookup implements the TranslationLookup interface -func (st *SemTable) ColumnLookup(col *sqlparser.ColName) (int, error) { +func (st *SemTable) ColumnLookup(*sqlparser.ColName) (int, error) { return 0, columnNotSupportedErr } diff --git a/go/vt/vtgate/semantics/table_set.go b/go/vt/vtgate/semantics/table_set.go index 0ddbc87a224..aa9042f9587 100644 --- a/go/vt/vtgate/semantics/table_set.go +++ b/go/vt/vtgate/semantics/table_set.go @@ -27,7 +27,7 @@ import ( type TableSet bitset.Bitset // Format formats the TableSet. -func (ts TableSet) Format(f fmt.State, verb rune) { +func (ts TableSet) Format(f fmt.State, _ rune) { first := true fmt.Fprintf(f, "TableSet{") bitset.Bitset(ts).ForEach(func(tid int) {