-
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
types: fix incorrect return type about if function when argument type contains bit #27611
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
TODO: Add some test |
… contains bit, fix bug and add some test
… contains bit, fix some test
… contains bit, fix test again
… contains bit, fix test again
/cc @lzmhhh123 @wshwsh12 |
…ix schema with wrong collation and charset, Add mock test
…ix schema with wrong collation and charset, rm mock test
@@ -1539,7 +1539,7 @@ func (s *testSuiteP2) TestUnion(c *C) { | |||
tk.MustExec("drop table if exists t") | |||
tk.MustExec("create table t(a bit(20), b float, c double, d int)") | |||
tk.MustExec("insert into t values(10, 10, 10, 10), (1, -1, 2, -2), (2, -2, 1, 1), (2, 1.1, 2.1, 10.1)") | |||
tk.MustQuery("select a from t union select 10 order by a").Check(testkit.Rows("1", "2", "10")) | |||
tk.MustQuery("select a from t union select 10 order by a").Check(testkit.Rows("\x00\x00\x01", "\x00\x00\x02", "\x00\x00\n", "10")) |
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 need change the test? Seems the old result same with mysql.
@@ -887,15 +887,15 @@ var fieldTypeMergeRules = [fieldTypeNum][fieldTypeNum]byte{ | |||
/* mysql.TypeBit -> */ | |||
{ | |||
// mysql.TypeUnspecified mysql.TypeTiny | |||
mysql.TypeVarchar, mysql.TypeLonglong, | |||
mysql.TypeVarchar, mysql.TypeVarchar, |
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.
Is there any reason for these changes? Mysql's type merge is same with tidb.
@yuqi1129: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I'm closing this PR as: 1) the related issue has been identified as a non-bug and 2) we don't see a respond to the review comment for a long time. Feel free to go to the original issue to discuss with us if you have any question. Thanks. |
… contains bit
What problem does this PR solve?
Issue Number: close #26358
Problem Summary:
What is changed and how it works?
Proposal: xxx
What's Changed:
How it Works:
Check List
Tests
Side effects
Documentation
Release note