Skip to content

Commit

Permalink
*: isolate more variables for runWithSystemSession (#54791)
Browse files Browse the repository at this point in the history
close #54343
  • Loading branch information
lance6716 authored Jul 23, 2024
1 parent 0bfbdc3 commit b19a918
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/executor/infoschema_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,6 @@ func (e *hugeMemTableRetriever) setDataForColumnsWithOneTable(
}

func (e *hugeMemTableRetriever) dataForColumnsInTable(ctx context.Context, sctx sessionctx.Context, schema *model.DBInfo, tbl *model.TableInfo, priv mysql.PrivilegeType, extractor *plannercore.ColumnsTableExtractor) {
is := sessiontxn.GetTxnManager(sctx).GetTxnInfoSchema()
if tbl.IsView() {
e.viewMu.Lock()
_, ok := e.viewSchemaMap[tbl.ID]
Expand All @@ -898,6 +897,7 @@ func (e *hugeMemTableRetriever) dataForColumnsInTable(ctx context.Context, sctx
internalCtx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnOthers)
// Build plan is not thread safe, there will be concurrency on sessionctx.
if err := runWithSystemSession(internalCtx, sctx, func(s sessionctx.Context) error {
is := sessiontxn.GetTxnManager(s).GetTxnInfoSchema()
planBuilder, _ := plannercore.NewPlanBuilder().Init(s.GetPlanCtx(), is, hint.NewQBHintHandler(nil))
var err error
viewLogicalPlan, err = planBuilder.BuildDataSourceFromView(ctx, schema.Name, tbl, nil, nil)
Expand Down
9 changes: 9 additions & 0 deletions pkg/executor/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import (
"github.com/pingcap/tidb/pkg/sessionctx/binloginfo"
"github.com/pingcap/tidb/pkg/sessionctx/sessionstates"
"github.com/pingcap/tidb/pkg/sessionctx/variable"
"github.com/pingcap/tidb/pkg/sessiontxn"
"github.com/pingcap/tidb/pkg/store/helper"
"github.com/pingcap/tidb/pkg/table"
"github.com/pingcap/tidb/pkg/table/tables"
Expand Down Expand Up @@ -2377,5 +2378,13 @@ func runWithSystemSession(ctx context.Context, sctx sessionctx.Context, fn func(
return err
}
defer b.ReleaseSysSession(ctx, sysCtx)
// `fn` may use KV transaction, so initialize the txn here
if err = sessiontxn.NewTxn(ctx, sysCtx); err != nil {
return err
}
defer sysCtx.RollbackTxn(ctx)
if err = ResetContextOfStmt(sysCtx, &ast.SelectStmt{}); err != nil {
return err
}
return fn(sysCtx)
}
2 changes: 2 additions & 0 deletions pkg/planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import (
"github.com/pingcap/tidb/pkg/util/set"
"github.com/pingcap/tidb/pkg/util/size"
"github.com/pingcap/tipb/go-tipb"
"go.uber.org/zap"
)

const (
Expand Down Expand Up @@ -5153,6 +5154,7 @@ func (b *PlanBuilder) BuildDataSourceFromView(ctx context.Context, dbName model.
}()
selectLogicalPlan, err := b.Build(ctx, selectNode)
if err != nil {
logutil.BgLogger().Error("build plan for view failed", zap.Error(err))
if terror.ErrorNotEqual(err, plannererrors.ErrViewRecursive) &&
terror.ErrorNotEqual(err, plannererrors.ErrNoSuchTable) &&
terror.ErrorNotEqual(err, plannererrors.ErrInternal) &&
Expand Down
4 changes: 4 additions & 0 deletions pkg/table/tables/tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,11 +690,15 @@ func TestViewColumns(t *testing.T) {
for _, testCase := range testCases {
tk.MustQuery(testCase.query).Check(testkit.RowsWithSep("|", testCase.expected...))
}
tk.MustExec("create view v1 as select (select a from t) as col from dual")
tk.MustQuery("select column_name, table_name from information_schema.columns where table_name='v1'").Check(
testkit.RowsWithSep("|", "col|v1"))
tk.MustExec("drop table if exists t")
for _, testCase := range testCases {
require.Len(t, tk.MustQuery(testCase.query).Rows(), 0)
tk.MustQuery("show warnings").Sort().Check(testkit.RowsWithSep("|",
"Warning|1356|View 'test.v' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them",
"Warning|1356|View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them",
"Warning|1356|View 'test.va' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them"))
}

Expand Down

0 comments on commit b19a918

Please sign in to comment.