expression: return error when doing ResolveIndices
#8929
Conversation
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
winoros
changed the title
ResolveIndicesResolveIndices
expression/column.go
Outdated
| col.Index = schema.ColumnIndex(col) | ||
| if col.Index == -1 { | ||
| log.Errorf("Can't find column %s in schema %s", col, schema) | ||
| return errors.Trace(errors.Errorf("Can't find column %s in schema %s", col, schema)) |
There was a problem hiding this comment.
Errorf won't add StackTree automatically.
And error stack is helpful for this kind of error.
There was a problem hiding this comment.
errors.Trace() will be removed in the near future. It's better to generate an error with stack and remove the errors.Trace() function call.
There was a problem hiding this comment.
errors.ErrorfWithStack(format string, args ...interface{})?
pingcap/errors have a method named WithStack(err) which i think it's the same thing with errors.Trace
There was a problem hiding this comment.
After reading the implementation of errors.Errorf(), I think the generated error already has a stack:
103 // Errorf formats according to a format specifier and returns the string
104 // as a value that satisfies error.
105 // Errorf also records the stack trace at the point it was called.
106 func Errorf(format string, args ...interface{}) error {
107 return &fundamental{
108 msg: fmt.Sprintf(format, args...),
109 stack: callers(),
110 }
111 }I think you can directly call errors.Errorf() to generate an error with stack trace.
There was a problem hiding this comment.
Oh, you're right. Changed
expression/column.go
Outdated
| col.Index = schema.ColumnIndex(col) | ||
| if col.Index == -1 { | ||
| log.Errorf("Can't find column %s in schema %s", col, schema) | ||
| return errors.Trace(errors.Errorf("Can't find column %s in schema %s", col, schema)) |
There was a problem hiding this comment.
errors.Trace() will be removed in the near future. It's better to generate an error with stack and remove the errors.Trace() function call.
winoros
force-pushed
the
resolve_indices
branch
from
ed57e85
to
965bb26
Compare
Codecov Report
@@ Coverage Diff @@
## master #8929 +/- ##
==========================================
- Coverage 67.25% 67.17% -0.08%
==========================================
Files 371 371
Lines 76223 76393 +170
==========================================
+ Hits 51261 51318 +57
- Misses 20424 20490 +66
- Partials 4538 4585 +47
Continue to review full report at Codecov.
|
alivxxx
added
status/LGT2
What problem does this PR solve?
Originally, there're some cases that
ResolveIndicesfind out that there're some columns not resolved although the SQL can be executed successfully.This pr fixes it and let
ResolveIndicesreturn error.What is changed and how it works?
Fix the following cases:
columnPruningdoesn't prune the_rowidcolumn correctly_rowidcolumn returned and changed the schema of operators above the DataSource operator.And changed the
ResolveIndicesmethod let it return an error.Check List
Tests
Code changes
Side effects
Related changes
None
This change is