Skip to content

Commit 043f833

Browse files
authored
executor: fix data race in nullmap in hash join v2 (#57451)
ref #53127, close #57450
1 parent a22b842 commit 043f833

File tree

2 files changed

+10
-24
lines changed

2 files changed

+10
-24
lines changed

pkg/executor/join/join_table_meta.go

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,12 @@ func newTableMeta(buildKeyIndex []int, buildTypes, buildKeyTypes, probeKeyTypes
211211
keyIndexMap := make(map[int]struct{})
212212
meta.serializeModes = make([]codec.SerializeMode, 0, len(buildKeyIndex))
213213
isAllKeyInteger := true
214-
hasFixedSizeKeyColumn := false
215214
varLengthKeyNumber := 0
216215
for index, keyIndex := range buildKeyIndex {
217216
keyType := buildKeyTypes[index]
218217
prop := getKeyProp(keyType)
219218
if prop.keyLength != chunk.VarElemLen {
220219
meta.joinKeysLength += prop.keyLength
221-
hasFixedSizeKeyColumn = true
222220
} else {
223221
meta.isJoinKeysFixedLength = false
224222
varLengthKeyNumber++
@@ -327,20 +325,10 @@ func newTableMeta(buildKeyIndex []int, buildTypes, buildKeyTypes, probeKeyTypes
327325
}
328326
if needUsedFlag {
329327
meta.colOffsetInNullMap = 1
330-
// the total row length should be larger than 4 byte since the smallest unit of atomic.LoadXXX is UInt32
331-
if len(columnsNeedToBeSaved) > 0 {
332-
// the smallest length of a column is 4 byte, so the total row length is enough
333-
meta.nullMapLength = (len(columnsNeedToBeSaved) + 1 + 7) / 8
334-
} else {
335-
// if no columns need to be converted to row format, then the key is not inlined
336-
// 1. if any of the key columns is fixed length, then the row length is larger than 4 bytes(since the smallest length of a fixed length column is 4 bytes)
337-
// 2. if all the key columns are variable length, there is no guarantee that the row length is larger than 4 byte, the nullmap should be 4 bytes alignment
338-
if hasFixedSizeKeyColumn {
339-
meta.nullMapLength = (len(columnsNeedToBeSaved) + 1 + 7) / 8
340-
} else {
341-
meta.nullMapLength = ((len(columnsNeedToBeSaved) + 1 + 31) / 32) * 4
342-
}
343-
}
328+
// If needUsedFlag == true, during probe stage, the usedFlag will be accessed by both read/write operator,
329+
// so atomic read/write is required. We want to keep this atomic operator inside the access of nullmap,
330+
// then the nullMapLength should be 4 bytes alignment since the smallest unit of atomic.LoadUint32 is UInt32
331+
meta.nullMapLength = ((len(columnsNeedToBeSaved) + 1 + 31) / 32) * 4
344332
} else {
345333
meta.colOffsetInNullMap = 0
346334
meta.nullMapLength = (len(columnsNeedToBeSaved) + 7) / 8

pkg/executor/join/join_table_meta_test.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -297,14 +297,12 @@ func TestJoinTableMetaNullMapLength(t *testing.T) {
297297
// nullmap only used for columns that needed to be converted to rows
298298
{[]int{0}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{}, false, 0},
299299
{[]int{0}, []*types.FieldType{stringTp, intTp, intTp, intTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{}, false, 0},
300-
// usedFlag is true
301-
// the row length is at least 4 bytes, so nullmap is 1 byte alignment
302-
{[]int{0}, []*types.FieldType{intTp}, []*types.FieldType{intTp}, []*types.FieldType{intTp}, nil, true, 1},
303-
{[]int{0}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{1}, true, 1},
304-
{[]int{0}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{0}, true, 1},
305-
{[]int{0, 1}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp, intTp}, []int{}, true, 1},
306-
{[]int{0}, []*types.FieldType{intTp}, []*types.FieldType{intTp}, []*types.FieldType{uintTp}, []int{}, true, 1},
307-
// the row length is not guaranteed to be at least 4 bytes, nullmap is 4 bytes alignment
300+
// usedFlag is true, nullmap is 4 byte alignment
301+
{[]int{0}, []*types.FieldType{intTp}, []*types.FieldType{intTp}, []*types.FieldType{intTp}, nil, true, 4},
302+
{[]int{0}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{1}, true, 4},
303+
{[]int{0}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{0}, true, 4},
304+
{[]int{0, 1}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp, intTp}, []*types.FieldType{stringTp, intTp}, []int{}, true, 4},
305+
{[]int{0}, []*types.FieldType{intTp}, []*types.FieldType{intTp}, []*types.FieldType{uintTp}, []int{}, true, 4},
308306
{[]int{0}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []*types.FieldType{stringTp}, []int{}, true, 4},
309307
{[]int{0, 1}, []*types.FieldType{stringTp, stringTp}, []*types.FieldType{stringTp, stringTp}, []*types.FieldType{stringTp, stringTp}, []int{}, true, 4},
310308
}

0 commit comments

Comments
 (0)