executor: fix bug of point get when meet null values

[alivxxx]

What problem does this PR solve?

For point get queries like select b, b from t where a = 1, if b is null, tidb will panic now.

What is changed and how it works?

The reason is that len(decodedVals[decodedPos]) will be 0 for null values, then it will not make reference for the second columns in the chunk, so it will panic when tidb tries to produce the result. This PR makes reference in the chunk for it.

Check List

Tests

• Unit test

Code changes

• Has exported function/method change

Side effects

• None

Related changes

• Need to cherry-pick to the release branch

[codecov]

Codecov Report

Merging #11219 into master will decrease coverage by 0.0157%.
The diff coverage is 84.6153%.

@@              Coverage Diff               @@
##             master   #11219        +/-   ##
==============================================
- Coverage   81.2958%   81.28%   -0.0159%     
==============================================
  Files           423      423                
  Lines         90071    90011        -60     
==============================================
- Hits          73224    73161        -63     
- Misses        11551    11553         +2     
- Partials       5296     5297         +1

[coocood]

This function looks complex, can we remove decodedPos2SchemaPos and range over e.schema.Columns to decode to chunk?

[winoros]

@coocood If we remove it, we still need to implement the same logic like this map since the column value can only be decoded once but it can be referenced multiple times?

[coocood]

@winoros
We can use col.ID to find the position in colID2DecodedPos, then get the value from the decoded row.

[alivxxx]

@coocood I think the main point of @winoros is if we need to decode one column several times, if not, then the code will not be cleaner. By the way, even if we can decode it several times, it does not make it cleaner because most of the codes are dealing with the different situation of columns.

[coocood]

@lamxTyler
Just decode as needed is straight forward, we don't need to optimize this case, it's a very rare case.

[coocood]

I think this is much simpler

func (e *PointGetExecutor) decodeRowValToChunk(rowVal []byte, chk *chunk.Chunk) error {
	//  One column could be filled for multi-times in the schema. e.g. select b, b, c, c from t where a = 1.
	// We need to set the positions in the schema for the same column.
	colID2CutPos := make(map[int64]int, e.schema.Len())
	for _, col := range e.schema.Columns {
		if _, ok := colID2CutPos[col.ID]; !ok {
			colID2CutPos[col.ID] = len(colID2CutPos)
		}
	}
	cutVals, err := tablecodec.CutRowNew(rowVal, colID2CutPos)
	if err != nil {
		return err
	}
	if cutVals == nil {
		cutVals = make([][]byte, len(colID2CutPos))
	}
	decoder := codec.NewDecoder(chk, e.ctx.GetSessionVars().Location())
	for colIdx, col := range e.schema.Columns {
		if e.tblInfo.PKIsHandle && mysql.HasPriKeyFlag(col.RetType.Flag) {
			chk.AppendInt64(colIdx, e.handle)
			continue
		}
		// ExtraHandleID is added when building plan, we can make sure that there's only one column's ID is this.
		if col.ID == model.ExtraHandleID {
			chk.AppendInt64(colIdx, e.handle)
			continue
		}
		cutVal := cutVals[colID2CutPos[col.ID]]
		if cutVal == nil {
			colInfo := getColInfoByID(e.tblInfo, col.ID)
			d, err1 := table.GetColOriginDefaultValue(e.ctx, colInfo)
			if err1 != nil {
				return err1
			}
			chk.AppendDatum(colIdx, &d)
			continue
		}
		_, err = decoder.DecodeOne(cutVal, colIdx, col.RetType)
		if err != nil {
			return err
		}
	}
	return nil
}

[coocood]

LGTM

[winoros]

lgtm

[alivxxx]

/run-all-tests