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

*: rewrite hex and bit literals #4415

Merged
merged 29 commits into from Sep 7, 2017

Conversation

Projects
None yet
6 participants
@breeswish
Member

breeswish commented Sep 4, 2017

In MySQL, Hex and Bit literals are strings (VAR_STRING) with unlimited byte length, while in numeric context (e.g. 0x1+0) they are integers with special cast-to-int rules other than normal strings: bytes are converted to uint in BigEndian, and if overflow it becomes MAXUINT. For BIT field, it is more like a int with special cast-to-string rules other than normal ints: the int value is transformed into bytes in BigEndian.

For the reasons above, this PR made the following changes:

  • Unify Hex literals and Bit literals at the Datum level. Previously Hex literals are stored as KindMysqlHex and Bit literals are KindMysqlBit. Now their datum kind is KindBinaryLiteral.
  • Store bytes instead of Uint in BinaryLiteral{} (so that Hex and Bit can store unlimited bytes).
  • In parser, Hex are parsed into HexLiteral{} and Bit are parsed into BitLiteral{} because we need to assign different flags for those literals after parsing. Both HexLiteral{} and BitLiteral{} are derived from BinaryLiteral{}. Notice that after storing into the datum, there is no HexLiteral{} and BitLiteral{} anymore: they are BinaryLiteral{}.
  • Separate BIT columns and Bit literals. Previously BIT columns and Bit literals are mixed together under KindMysqlBit. Now, for BIT columns, its datum kind is KindMysqlBit, while for BIT literals its datum kind is KindBinaryLiteral (and its field type is TypeVarString). Data in KindMysqlBit are stored in the same format as KindBinaryLiterals though (i.e. []byte in BigEndian), so that utilities in BinaryLiteral{} can be reused.

KindMysqlBit are serialized to/from uint in table encoding and table decoding (the same as previous).

Fix #4374
Fix #4355
Fix #4432
Fix #4428

TODO Fix: Field Type Bit got wrong result in php-mysqli.
Bug can be reproduced in master. Opened another issue to handle this: #4428 Fixed in this PR as well.

@breeswish breeswish added the status/WIP label Sep 4, 2017

c.Assert(err, NotNil)
// src = "select 0b'';"
// _, err = parser.ParseOneStmt(src, "", "")
// c.Assert(err, NotNil)

This comment has been minimized.

@breeswish

breeswish Sep 4, 2017

Member

Will be fixed later.

@breeswish

breeswish Sep 4, 2017

Member

Will be fixed later.

This comment has been minimized.

@breeswish

breeswish Sep 4, 2017

Member

@winkyao Could you take a look at this? The error assertion failed: it does not throw errors. However in BinString, such situation (0b'') is already handled and the test passed: https://github.com/pingcap/tidb/pull/4415/files#diff-47c568d11d2c996af704b03f8c1c1203R57

@breeswish

breeswish Sep 4, 2017

Member

@winkyao Could you take a look at this? The error assertion failed: it does not throw errors. However in BinString, such situation (0b'') is already handled and the test passed: https://github.com/pingcap/tidb/pull/4415/files#diff-47c568d11d2c996af704b03f8c1c1203R57

@breeswish breeswish changed the title from [WIP] *: rewrite hex and bit literals to *: rewrite hex and bit literals Sep 4, 2017

@breeswish breeswish added status/DNM and removed status/WIP labels Sep 4, 2017

@breeswish breeswish changed the title from *: rewrite hex and bit literals to [DNM] *: rewrite hex and bit literals Sep 4, 2017

@breeswish breeswish changed the title from [DNM] *: rewrite hex and bit literals to *: rewrite hex and bit literals Sep 4, 2017

@breeswish breeswish removed the status/DNM label Sep 4, 2017

breeswish added some commits Sep 4, 2017

@iamxy

This comment has been minimized.

Show comment
Hide comment
@iamxy

iamxy Sep 5, 2017

Member

/build

Member

iamxy commented Sep 5, 2017

/build

breeswish added some commits Sep 5, 2017

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish
Member

breeswish commented Sep 5, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Sep 5, 2017

Member

/run-all-test

Member

zz-jason commented Sep 5, 2017

/run-all-test

Show outdated Hide outdated util/types/binstring.go
Show outdated Hide outdated util/types/binstring.go
Show outdated Hide outdated util/types/binstring.go
Show outdated Hide outdated util/types/binstring_test.go
Show outdated Hide outdated util/types/datum.go

@zz-jason zz-jason added the status/LGT1 label Sep 5, 2017

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish

breeswish Sep 5, 2017

Member

@zz-jason Renamed BinString into BinaryLiteral.

Member

breeswish commented Sep 5, 2017

@zz-jason Renamed BinString into BinaryLiteral.

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Sep 6, 2017

Contributor

PTAL @coocood

Contributor

XuHuaiyu commented Sep 6, 2017

PTAL @coocood

Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/binary_literal.go
Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/datum_eval.go
Show outdated Hide outdated tablecodec/tablecodec.go
Show outdated Hide outdated util/codec/codec.go
Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/datum.go
Show outdated Hide outdated util/types/binary_literal.go

breeswish added some commits Sep 7, 2017

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish
Member

breeswish commented Sep 7, 2017

@coocood Done

breeswish added some commits Sep 7, 2017

Merge branches 'wenxuan/grumble-hex-bit' and 'wenxuan/grumble-hex-bit…
…' of github.com:pingcap/tidb into wenxuan/grumble-hex-bit
@coocood

coocood approved these changes Sep 7, 2017

LGTM

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Sep 7, 2017

Member

/run-all-test

Member

coocood commented Sep 7, 2017

/run-all-test

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Sep 7, 2017

Member

/run-all-test

Member

zz-jason commented Sep 7, 2017

/run-all-test

@zz-jason zz-jason added status/LGT2 and removed status/LGT1 labels Sep 7, 2017

@hanfei1991 hanfei1991 merged commit fc209cb into master Sep 7, 2017

10 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
jenkins-ci-tidb/common-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
jenkins-ci-tidb/mybatis-test Jenkins job succeeded.
Details
jenkins-ci-tidb/sqllogic-test Jenkins job succeeded.
Details
jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@hanfei1991 hanfei1991 deleted the wenxuan/grumble-hex-bit branch Sep 7, 2017

mccxj added a commit to mccxj/tidb that referenced this pull request Sep 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment