-
Notifications
You must be signed in to change notification settings - Fork 25.6k
SQL: Fix column size for IP data type #53056
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
Conversation
Set size/displaySize to 45 which is the maximum string for an IP (v6), since IPs are returned as strings. Fixes: elastic#52762
|
Pinging @elastic/es-search (:Search/SQL) |
costin
left a comment
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
| } | ||
| if (dataType == IP) { | ||
| return 39; | ||
| return 45; |
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.
DataType.size() could be potentially returned since it's the same value.
| if (dataType == DATETIME) { | ||
| return 29; | ||
| } | ||
| if (dataType == IP) { |
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.
DataType.size() could be potentially returned since it's the same value.
In fact, maybe this approach could be used for other DataTypes as well if applicable - that is whenever size == displaySize
|
No need to backport all the way to 6.x |
bpintea
left a comment
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
astefan
left a comment
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
Set size/displaySize to
45which is the maximum string foran IP (v6), since IPs are returned as strings.
Fixes: #52762