expression: resize the result for IfXXSig #37417

XuHuaiyu · 2022-08-27T16:09:40Z

What problem does this PR solve?

Issue Number: close #37414

Problem Summary:
The IfXXSig.fallbackEvalXX forgets to resize the result, thus trigger the index out of range error.

What is changed and how it works?

Check List

Tests

Unit test

Side effects

N/A

Documentation

N/A

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2022-08-27T16:09:42Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

qw4990
wshwsh12

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

fixdb

Will other expressions like case when have the same issue?

fixdb · 2022-08-27T17:09:10Z

I am not able to reproduce the issue with case when though:

insert into foo select case when b>0 then  
  case when a/b>1 then  1 else  a/b end 
 else null end 
from bar;

I am not sure whether it doesn't have the issue or it just can't expose the issue.

gengliqi · 2022-08-28T06:15:15Z

expression/integration_test.go

+	tk.MustExec("create table foo(a decimal(65,0));")
+	tk.MustExec("create table bar(a decimal(65,0), b decimal(65,0));")
+	tk.MustExec("insert into bar values(0,0),(1,1),(2,2);")
+	tk.MustExec("insert into foo select if(b>0, if(a/b>1, 1, 2), null) from bar;")


How about adding ifNULL and CaseWhen cases?

gengliqi · 2022-08-28T06:17:24Z

I am not able to reproduce the issue with case when though:
insert into foo select case when b>0 then  
  case when a/b>1 then  1 else  a/b end 
 else null end 
from bar;
I am not sure whether it doesn't have the issue or it just can't expose the issue.

case when doesn't have this issue. See line 67 in the below code.

tidb/expression/generator/control_vec.go

Lines 64 to 69 in a8f524b

    
           func (b *builtinCaseWhen{{ .TypeName }}Sig) fallbackEval{{ .TypeName }}(input *chunk.Chunk, result *chunk.Column) error { 
        
           	n := input.NumRows() 
        
           	{{- if .Fixed }} 
        
           	result.Resize{{ .TypeNameInColumn }}(n, false) 
        
           	x := result.{{ .TypeNameInColumn }}s() 
        
           	for i := 0; i < n; i++ {

XuHuaiyu · 2022-08-29T03:15:09Z

IfNull does not have this problem, neither.
result will be resized in line 271 when evaluating arg[0].VecEval.

tidb/expression/generator/control_vec.go

Lines 268 to 277 in 7425d2f

    
           func (b *builtinIfNull{{ .TypeName }}Sig) vecEval{{ .TypeName }}(input *chunk.Chunk, result *chunk.Column) error { 
        
           	n := input.NumRows() 
        
           	{{- if .Fixed }} 
        
           	if err := b.args[0].VecEval{{ .TypeName }}(b.ctx, input, result); err != nil { 
        
           		return err 
        
           	} 
        
           	buf1, err := b.bufAllocator.get() 
        
           	if err != nil { 
        
           		return err 
        
           	}

wshwsh12 · 2022-08-29T03:36:57Z

/merge

ti-chi-bot · 2022-08-29T03:37:00Z

This pull request has been accepted and is ready to merge.

Commit hash: af1047f

ti-srebot · 2022-08-29T04:20:17Z

cherry pick to release-5.0 in PR #37425