Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL failed with "ERROR 1105 (HY000): runtime error: invalid memory address or nil pointer dereference" when tidb_enable_inl_join_inner_multi_pattern is ON #55169

Closed
joechenrh opened this issue Aug 2, 2024 · 2 comments · Fixed by #55201
Assignees
Labels
impact/panic severity/major sig/planner SIG: Planner type/bug The issue is confirmed as a bug.

Comments

@joechenrh
Copy link
Contributor

Bug Report

1. Minimal reproduce step (Required)

set GLOBAL tidb_enable_inl_join_inner_multi_pattern='ON';
create table t1(col_1 int, index idx_1(col_1));
create table t2(col_1 int, col_2 int, index idx_2(col_1));

select /*+ inl_join(tmp) */ * from t1 inner join (select col_1, group_concat(col_2) from t2 group by col_1) tmp on t1.col_1 = tmp.col_1;

2. What did you expect to see? (Required)

Run successfully

3. What did you see instead (Required)

[2024/08/02 15:51:11.915 +08:00] [ERROR] [conn.go:1040] ["connection running loop panic"] [conn=2097156] [session_alias=] [lastSQL="select /*+ inl_join(tmp) */ * from t1 inner join (select col_1, group_concat(col_2) from t2 group by col_1) tmp on t1.col_1 = tmp.col_1"] [err="runtime error: invalid memory address or nil pointer dereference"] [stack="github.com/pingcap/tidb/pkg/server.(*clientConn).Run.func1\n\t/Users/joechenrh/code/tidb-fix/pkg/server/conn.go:1043\nruntime.gopanic
/Users/joechenrh/deps/go/src/runtime/panic.go:770\ngit.luolix.top/pingcap/tidb/pkg/executor.(*Compiler).Compile.func1
/Users/joechenrh/code/tidb-fix/pkg/executor/compiler.go:57\nruntime.gopanic
/Users/joechenrh/deps/go/src/runtime/panic.go:770\nruntime.panicmem
/Users/joechenrh/deps/go/src/runtime/panic.go:261\nruntime.sigpanic
/Users/joechenrh/deps/go/src/runtime/signal_unix.go:881\ngit.luolix.top/pingcap/tidb/pkg/planner/property.(*PhysicalProperty).CloneEssentialFields
/Users/joechenrh/code/tidb-fix/pkg/planner/property/physical_property.go:366\ngit.luolix.top/pingcap/tidb/pkg/planner/core.InjectProjBelowAgg
/Users/joechenrh/code/tidb-fix/pkg/planner/core/rule_inject_extra_projection.go:208\ngit.luolix.top/pingcap/tidb/pkg/planner/core.(*projInjector).inject
/Users/joechenrh/code/tidb-fix/pkg/planner/core/rule_inject_extra_projection.go:68\ngit.luolix.top/pingcap/tidb/pkg/planner/core.(*projInjector).inject\n\t/Users/joechenrh/code/tidb-fix/pkg/planner/core/rule_inject_extra_projection.go:58\ngit.luolix.top/pingcap/tidb/pkg/planner/core.(*projInjector).inject
/Users/joechenrh/code/tidb-fix/pkg/planner/core/rule_inject_extra_projection.go:58\ngit.luolix.top/pingcap/tidb/pkg/planner/core.InjectExtraProjection
/Users/joechenrh/code/tidb-fix/pkg/planner/core/rule_inject_extra_projection.go:45\ngit.luolix.top/pingcap/tidb/pkg/planner/core.postOptimize
/Users/joechenrh/code/tidb-fix/pkg/planner/core/optimizer.go:424\ngit.luolix.top/pingcap/tidb/pkg/planner/core.doOptimize
/Users/joechenrh/code/tidb-fix/pkg/planner/core/optimizer.go:312\ngit.luolix.top/pingcap/tidb/pkg/planner/core.DoOptimize
/Users/joechenrh/code/tidb-fix/pkg/planner/core/optimizer.go:355\ngit.luolix.top/pingcap/tidb/pkg/planner.optimize
/Users/joechenrh/code/tidb-fix/pkg/planner/optimize.go:525\ngit.luolix.top/pingcap/tidb/pkg/planner.Optimize
/Users/joechenrh/code/tidb-fix/pkg/planner/optimize.go:356\ngit.luolix.top/pingcap/tidb/pkg/executor.(*Compiler).Compile
/Users/joechenrh/code/tidb-fix/pkg/executor/compiler.go:99\ngit.luolix.top/pingcap/tidb/pkg/session.(*session).ExecuteStmt
/Users/joechenrh/code/tidb-fix/pkg/session/session.go:2097\ngit.luolix.top/pingcap/tidb/pkg/server.(*TiDBContext).ExecuteStmt
/Users/joechenrh/code/tidb-fix/pkg/server/driver_tidb.go:291\ngit.luolix.top/pingcap/tidb/pkg/server.(*clientConn).handleStmt
/Users/joechenrh/code/tidb-fix/pkg/server/conn.go:2047\ngit.luolix.top/pingcap/tidb/pkg/server.(*clientConn).handleQuery
/Users/joechenrh/code/tidb-fix/pkg/server/conn.go:1801\ngit.luolix.top/pingcap/tidb/pkg/server.(*clientConn).dispatch
/Users/joechenrh/code/tidb-fix/pkg/server/conn.go:1375\ngit.luolix.top/pingcap/tidb/pkg/server.(*clientConn).Run
/Users/joechenrh/code/tidb-fix/pkg/server/conn.go:1141\ngit.luolix.top/pingcap/tidb/pkg/server.(*Server).onConn
/Users/joechenrh/code/tidb-fix/pkg/server/server.go:740"]

4. What is your TiDB version? (Required)

@joechenrh joechenrh added the type/bug The issue is confirmed as a bug. label Aug 2, 2024
@joechenrh
Copy link
Contributor Author

var aggTask base.Task
// build stream agg and change ds keep order to true
if preferStream {
newGbyItems := make([]expression.Expression, len(la.GroupByItems))
copy(newGbyItems, la.GroupByItems)
newAggFuncs := make([]*aggregation.AggFuncDesc, len(la.AggFuncs))
copy(newAggFuncs, la.AggFuncs)
streamAgg := basePhysicalAgg{
GroupByItems: newGbyItems,
AggFuncs: newAggFuncs,
}.initForStream(la.SCtx(), la.StatsInfo(), la.QueryBlockOffset(), nil)
streamAgg.SetSchema(la.Schema().Clone())
// change to keep order for index scan and dsCopTask
if dsCopTask.indexPlan != nil {
// get the index scan from dsCopTask.indexPlan
physicalIndexScan, _ := dsCopTask.indexPlan.(*PhysicalIndexScan)
if physicalIndexScan == nil && len(dsCopTask.indexPlan.Children()) == 1 {
physicalIndexScan, _ = dsCopTask.indexPlan.Children()[0].(*PhysicalIndexScan)
}
if physicalIndexScan != nil {
physicalIndexScan.KeepOrder = true
dsCopTask.keepOrder = true
aggTask = streamAgg.Attach2Task(dsCopTask)
}
}
}
// build hash agg, when the stream agg is illegal such as the order by prop is not matched
if aggTask == nil {
physicalHashAgg := NewPhysicalHashAgg(la, la.StatsInfo(), nil)
physicalHashAgg.SetSchema(la.Schema().Clone())
aggTask = physicalHashAgg.Attach2Task(dsCopTask)
}

For those aggregation functions that cannot be pushed down, their PhysicalProperty is not set.

Add a default property &property.PhysicalProperty{ExpectedCnt: math.MaxFloat64} like it does in newPartialAggregate() seems work.

@joechenrh
Copy link
Contributor Author

Another panic

select /*+ inl_join(tmp) */ * from t1 inner join (select col_1, group_concat(distinct col_2 order by col_2) from t2 group by col_1) tmp on t1.col_1 = tmp.col_1;

PhysicalProperty is not set in this conditional branch too.

if !canPushAggToCop {
result := dsCopTask.ConvertToRootTask(ds.SCtx()).(*RootTask)
result.SetPlan(constructInnerByZippedChildren(wrapper.zippedChildren, result.GetPlan()))
return result
}

@hawkingrei hawkingrei added affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. and removed may-affects-8.1 labels Aug 6, 2024
@hawkingrei hawkingrei removed may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 may-affects-6.5 labels Aug 6, 2024
@ti-chi-bot ti-chi-bot bot closed this as completed in de943d1 Aug 6, 2024
@winoros winoros removed the affects-7.1 This bug affects the 7.1.x(LTS) versions. label Sep 20, 2024
@winoros winoros removed affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. labels Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/panic severity/major sig/planner SIG: Planner type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants