-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
infoschema: refine infoschema's MaxLength Precision value #7463
Conversation
/run-all-tests |
/run-common-test tidb-test=pr/611 |
@@ -792,8 +792,12 @@ func dataForColumns(ctx sessionctx.Context, schemas []*model.DBInfo) [][]types.D | |||
func dataForColumnsInTable(schema *model.DBInfo, tbl *model.TableInfo) [][]types.Datum { | |||
var rows [][]types.Datum | |||
for i, col := range tbl.Columns { | |||
var charMaxLen, charOctLen, numericPrecision, numericScale, datetimePrecision interface{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use int instead of interface{}? Are there any benefits of using interface{} here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using interface{}
, we can got NULL
result~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface is really slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes~~we should void using interface{}
as we can, but in this case is special, types.MakeDatums
accept args ...interface{}
as arguments.
cast to interface always happen, so direct using interface{}
local variable is simple.
we also can use *int
local varible instead of interface{}
, but that will make us step into a well-known trap in go https://play.golang.org/p/qw6UJhvZcrR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
fixes #7396
What is changed and how it works?
set value for
CHARACTER_MAXIMUM_LENGTH
,CHARACTER_OCTET_LENGTH
,NUMERIC_PRECISION
,NUMERIC_SCALE
,DATETIME_PRECISION
base on different column type.see more example in new added TestCase
Tests
Code changes
Side effects
Related changes
This change is