-
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
expression: fix type infer for tidb's builtin compare(least and greatest) #21150
Conversation
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
types/field_type.go
Outdated
@@ -1258,6 +1262,49 @@ var fieldTypeMergeRules = [fieldTypeNum][fieldTypeNum]byte{ | |||
}, | |||
} | |||
|
|||
const ( | |||
InvalidResult byte = 0 /** not valid for UDFs */ |
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.
Maybe you need to define a new type for these constants?
types/field_type.go
Outdated
StringResult byte = 1 /** char * */ | ||
RealResult byte = 2 /** double */ | ||
IntResult byte = 3 /** long long */ | ||
RowResult byte = 4 /** not valid for UDFs */ |
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.
Unused?
types/field_type.go
Outdated
RealResult byte = 2 /** double */ | ||
IntResult byte = 3 /** long long */ | ||
RowResult byte = 4 /** not valid for UDFs */ | ||
DecimalResult byte = 5 /** char *, to be converted to/from a decimal */ |
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.
There is no char *
in TiDB?
types/field_type.go
Outdated
StringResult, StringResult, | ||
// mysql.TypeBit mysql.TypeInvalid | ||
IntResult, InvalidResult, | ||
// Unused entries: <17>-<242> |
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.
In previous definitions, the matrix order is TypeBit, <16>-<244>, TypeJSON, ...
In this implementation, the order is TypeBit, TypeInvalid, <17>-<242>, TypeBool, TypeJSON
Array elements are no longer aligned. Will there be problems?
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
/cc @lzmhhh123 |
/reward 900 |
This PR's linked issue is not picked. |
/run-all-tests |
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus Should it be cherry-picked into release-3.0 and release-4.0? |
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@lzmhhh123 @breeswish PTAL |
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
expression/builtin_compare.go
Outdated
|
||
var temporalItem *types.FieldType | ||
aggType.EvalType() | ||
if types.ResultMergeType(aggType.Tp) == types.ETString { |
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.
we can check whether the ETType == ETDatetime || ETType == ETString || ...
instead of using ResultMergeType here.
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
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
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
…nto fix-least-and-greatest
/run-all-tests tidb-test=pr/1137 |
/run-unit-test |
/merge |
/run-all-tests |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-4.0 in PR #21924 |
…est) (pingcap#21150) Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
Signed-off-by: iosmanthus myosmanthustree@gmail.com
What problem does this PR solve?
Issue Number:
close #17994
close #9765
Problem Summary:
What is changed and how it works?
What's Changed:
This pull request migrates some MySQL type merge rules to make TiDB's type infer rules more MySQL compatible.
How it Works:
This pull request port https://github.com/mysql/mysql-server/blob/ee4455a33b10f1b1886044322e4893f587b319ed/sql%2Fitem_func.cc#L3470 to infer the result type of
least/greatest
Related changes
pingcap/docs
/pingcap/docs-cn
:Check List
Tests
Side effects
Release note