Repository: matrixorigin/matrixone
Pull request #13441 (2023-12-13 09:46:26)

Fix the wrong compare of catalog cache's mvcc
## What type of PR is this?

- [ ] API-change
- [X] BUG
- [ ] Improvement
- [ ] Documentation
- [ ] Feature
- [ ] Test and CI
- [ ] Code Refactoring

## Which issue(s) this PR fixes:

issue  #https://github.com/matrixorigin/MO-Cloud/issues/2009

## What this PR does / why we need it:
对于类似于以下场景:
```sql
begin;
create table t(a int);
alter table t add column `remark` varchar(100);
commit;
```
会出现表找不到的情况，本质原因是因为上述事务会产生以下logtail:
```
k0 = insert (t(name)+10(tableid) + 100(timestamp))
k1 = delete (t(name)+10(tableid) + 100(timestamp))
k2 = insert (t(name)+12(tableid) + 100(timestamp))
```
因为多版本k的比较代码存在如下的代码:
```go
if a.deleted && !b.deleted { //deleted item head first
			return true
}
```
所以会生成k1, k2, k0类似于这样的序列，这样的情况会导致表找不到。修改为delete只比需要删除的key大来修正这个问题


Patch:
diff --git a/pkg/vm/engine/disttae/cache/catalog.go b/pkg/vm/engine/disttae/cache/catalog.go
index 22cd1e5ed..955b09f36 100644
--- a/pkg/vm/engine/disttae/cache/catalog.go
+++ b/pkg/vm/engine/disttae/cache/catalog.go
@@ -16,11 +16,12 @@ package cache
 
 import (
 	"fmt"
-	plan2 "github.com/matrixorigin/matrixone/pkg/sql/plan"
-	"github.com/matrixorigin/matrixone/pkg/sql/util"
 	"math"
 	"sort"
 
+	plan2 "github.com/matrixorigin/matrixone/pkg/sql/plan"
+	"github.com/matrixorigin/matrixone/pkg/sql/util"
+
 	"github.com/matrixorigin/matrixone/pkg/catalog"
 	"github.com/matrixorigin/matrixone/pkg/compress"
 	"github.com/matrixorigin/matrixone/pkg/container/batch"
@@ -208,6 +209,7 @@ func (cc *CatalogCache) GetTable(tbl *TableItem) bool {
 	var tableId uint64
 	deleted := make(map[uint64]bool)
 	inserted := make(map[uint64]*TableItem)
+	tbl.Id = math.MaxUint64
 	cc.tables.data.Ascend(tbl, func(item *TableItem) bool {
 		if item.deleted && item.AccountId == tbl.AccountId &&
 			item.DatabaseId == tbl.DatabaseId && item.Name == tbl.Name {
diff --git a/pkg/vm/engine/disttae/cache/types.go b/pkg/vm/engine/disttae/cache/types.go
index 6d6d89a19..6acf43888 100644
--- a/pkg/vm/engine/disttae/cache/types.go
+++ b/pkg/vm/engine/disttae/cache/types.go
@@ -225,13 +225,16 @@ func tableItemLess(a, b *TableItem) bool {
 			Then the item x3 will be lost. The TN will not know the table x3. It is wrong!
 			With sort on the table id, the item x3,x2,x1 will be reserved.
 		*/
-		if a.deleted && !b.deleted { //deleted item head first
+		// the logtail is unordered, so we need to sort the items by table id.
+		// the larger table id is created later.
+		if a.Id > b.Id {
 			return true
-		} else if !a.deleted && b.deleted {
+		}
+		if a.Id < b.Id {
 			return false
-		} else { //a.deleted && b.deleted || !a.deleted && !b.deleted
-			return a.Id < b.Id
 		}
+		// for the same table id, the delete item is head.
+		return a.deleted
 	}
 	return a.Ts.Greater(b.Ts)
 }


Test patch:
diff --git a/test/distributed/cases/pessimistic_transaction/create_table.result b/test/distributed/cases/pessimistic_transaction/create_table.result
new file mode 100644
index 000000000..418619269
--- /dev/null
+++ b/test/distributed/cases/pessimistic_transaction/create_table.result
@@ -0,0 +1,10 @@
+create database if not exists test;
+use test;
+drop table if exists t;
+begin;
+create table t(a int);
+alter table t add column b int;
+commit;
+select * from t;
+a    b
+drop database test;
diff --git a/test/distributed/cases/pessimistic_transaction/create_table.sql b/test/distributed/cases/pessimistic_transaction/create_table.sql
new file mode 100644
index 000000000..75f000185
--- /dev/null
+++ b/test/distributed/cases/pessimistic_transaction/create_table.sql
@@ -0,0 +1,9 @@
+create database if not exists test;
+use test;
+drop table if exists t;
+begin;
+create table t(a int);
+alter table t add column b int;
+commit;
+select * from t;
+drop database test;