Skip to content
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

*: fix in-compatible behavior when modify value from Navicat GUI #6105

Merged
merged 5 commits into from Mar 22, 2018

Conversation

XuHuaiyu
Copy link
Contributor

@XuHuaiyu XuHuaiyu commented Mar 21, 2018

This commit fix 2 things which will affect modify values from Navicat GUI,
and remove comparison of FieldType.Flag when mergeFiledType for Union. :

  1. add syntax support for Group_concat( order by)
    Before this commit:
tidb(127.0.0.1)> SELECT student_name, GROUP_CONCAT(DISTINCT test_score ORDER BY test_score DESC SEPARATOR ' ') FROM
 student GROUP BY student_name;
ERROR 1105 (HY000): line 1 column 59 near " BY test_score DESC SEPARATOR ' ') FROM student GROUP BY student_name" (
total length 128)
  1. fix the type infer of numeric type columns
    Before this commit:
create table t1(a int, b decimal(15,2));
select * from t1;
MySQL:

Field   1:  `a`
Catalog:    `def`
Database:   `test`
Table:      `t1`
Org_table:  `t1`
Type:       LONG
Collation:  binary (63)
Length:     11
Max_length: 0
Decimals:   0
Flags:      NUM       <== No BinaryFlag

Field   2:  `b`
Catalog:    `def`
Database:   `test`
Table:      `t1`
Org_table:  `t1`
Type:       NEWDECIMAL
Collation:  binary (63)
Length:     17
Max_length: 0
Decimals:   2
Flags:      NUM       <== No BinaryFlag
TiDB:

Field   1:  `a`
Catalog:    `def`
Database:   `test`
Table:      `t1`
Org_table:  `t1`
Type:       LONG
Collation:  binary (63)
Length:     44
Max_length: 0
Decimals:   0
Flags:      BINARY NUM     

Field   2:  `b`
Catalog:    `def`
Database:   `test`
Table:      `t1`
Org_table:  `t1`
Type:       NEWDECIMAL
Collation:  binary (63)
Length:     17
Max_length: 0
Decimals:   2
Flags:      BINARY NUM      

@XuHuaiyu
Copy link
Contributor Author

PTAL @zz-jason @coocood

@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

@@ -55,7 +55,7 @@ func NewFieldType(tp byte) *FieldType {
// Equal checks whether two FieldType objects are equal.
func (ft *FieldType) Equal(other *FieldType) bool {
partialEqual := ft.Tp == other.Tp &&
ft.Flag == other.Flag &&
//ft.Flag == other.Flag && // We do not need to compare this when wrap cast upon an Expression.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is strange to comment inside a statement. Could we explain this above line 57?

@XuHuaiyu
Copy link
Contributor Author

PTAL @jackysp

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@XuHuaiyu XuHuaiyu added status/LGT1 Indicates that a PR has LGTM 1. all-tests-passed labels Mar 22, 2018
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@XuHuaiyu XuHuaiyu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 22, 2018
@XuHuaiyu XuHuaiyu merged commit 5445e17 into pingcap:master Mar 22, 2018
@XuHuaiyu XuHuaiyu deleted the navicat branch March 22, 2018 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2. type/compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants