From 3c7d7139500d6dbc4711f7f8dcbdd62891a2e2be Mon Sep 17 00:00:00 2001
From: ti-srebot <66930949+ti-srebot@users.noreply.github.com>
Date: Mon, 22 Aug 2022 10:34:53 +0800
Subject: [PATCH] executor: fix left join on partition table generate invalid
 lock key (#35732) (#35774)

close pingcap/tidb#28073
---
 executor/executor.go        |  9 +++++++++
 executor/union_scan_test.go | 24 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/executor/executor.go b/executor/executor.go
index f903335a91fdf..9aba519361175 100644
--- a/executor/executor.go
+++ b/executor/executor.go
@@ -1025,6 +1025,15 @@ func (e *SelectLockExec) Next(ctx context.Context, req *chunk.Chunk) error {
 					physTblID := tblID
 					if physTblColIdx, ok := e.tblID2PhysTblIDColIdx[tblID]; ok {
 						physTblID = row.GetInt64(physTblColIdx)
+						if physTblID == 0 {
+							// select * from t1 left join t2 on t1.c = t2.c for update
+							// The join right side might be added NULL in left join
+							// In that case, physTblID is 0, so skip adding the lock.
+							//
+							// Note, we can't distinguish whether it's the left join case,
+							// or a bug that TiKV return without correct physical ID column.
+							continue
+						}
 					}
 					e.keys = append(e.keys, tablecodec.EncodeRowKeyWithHandle(physTblID, handle))
 				}
diff --git a/executor/union_scan_test.go b/executor/union_scan_test.go
index 4979af65273bd..b585ba5f20c13 100644
--- a/executor/union_scan_test.go
+++ b/executor/union_scan_test.go
@@ -15,11 +15,13 @@
 package executor_test
 
 import (
+	"encoding/hex"
 	"fmt"
 	"testing"
 	"time"
 
 	"github.com/pingcap/tidb/executor"
+	"github.com/pingcap/tidb/store/helper"
 	"github.com/pingcap/tidb/tablecodec"
 	"github.com/pingcap/tidb/testkit"
 	"github.com/pingcap/tidb/util/benchdaily"
@@ -456,6 +458,28 @@ func TestIssue28073(t *testing.T) {
 		break
 	}
 	require.False(t, exist)
+
+	// Another case, left join on partition table should not generate locks on physical ID = 0
+	tk.MustExec("drop table if exists t1, t2;")
+	tk.MustExec("create table t1  (c_int int, c_str varchar(40), primary key (c_int, c_str));")
+	tk.MustExec("create table t2  (c_int int, c_str varchar(40), primary key (c_int)) partition by hash (c_int) partitions 4;")
+	tk.MustExec("insert into t1 (`c_int`, `c_str`) values (1, 'upbeat solomon'), (5, 'sharp rubin');")
+	tk.MustExec("insert into t2 (`c_int`, `c_str`) values (1, 'clever haibt'), (4, 'kind margulis');")
+	tk.MustExec("begin pessimistic;")
+	tk.MustQuery("select * from t1 left join t2 on t1.c_int = t2.c_int for update;").Check(testkit.Rows(
+		"1 upbeat solomon 1 clever haibt",
+		"5 sharp rubin <nil> <nil>",
+	))
+	key, err := hex.DecodeString("7480000000000000005F728000000000000000")
+	require.NoError(t, err)
+	h := helper.NewHelper(store.(helper.Storage))
+	resp, err := h.GetMvccByEncodedKey(key)
+	require.NoError(t, err)
+	require.Nil(t, resp.Info.Lock)
+	require.Len(t, resp.Info.Writes, 0)
+	require.Len(t, resp.Info.Values, 0)
+
+	tk.MustExec("rollback;")
 }
 
 func TestIssue32422(t *testing.T) {