Skip to content

Differentiate between BINARY and CHAR #724

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

Merged

Conversation

kwoodhouse93
Copy link
Contributor

When looking up the database type name, we now check the character set
for the following field types:

  • CHAR
  • VARCHAR
  • BLOB
  • TINYBLOB
  • MEDIUMBLOB
  • LONGBLOB

If the character set is 63 (which is the binary pseudo character set),
we return the binary names, which are (respectively):

  • BINARY
  • VARBINARY
  • BLOB
  • TINYBLOB
  • MEDIUMBLOB
  • LONGBLOB

If any other character set is in use, we return the text names, which
are (again, respectively):

  • CHAR
  • VARCHAR
  • TEXT
  • TINYTEXT
  • MEDIUMTEXT
  • LONGTEXT

To facilitate this, mysqlField has been extended to include a uint8
field for character set, which is read from the appropriate packet.

Column type tests have been updated to ensure coverage of binary and
text types.

Description

This is a pull request to support differentiating between BINARY and CHAR, as discussed under issue #723.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

Kieron Woodhouse added 2 commits December 21, 2017 10:02
When looking up the database type name, we now check the character set
for the following field types:
 * CHAR
 * VARCHAR
 * BLOB
 * TINYBLOB
 * MEDIUMBLOB
 * LONGBLOB

If the character set is 63 (which is the binary pseudo character set),
we return the binary names, which are (respectively):
 * BINARY
 * VARBINARY
 * BLOB
 * TINYBLOB
 * MEDIUMBLOB
 * LONGBLOB

If any other character set is in use, we return the text names, which
are (again, respectively):
 * CHAR
 * VARCHAR
 * TEXT
 * TINYTEXT
 * MEDIUMTEXT
 * LONGTEXT

To facilitate this, mysqlField has been extended to include a uint8
field for character set, which is read from the appropriate packet.

Column type tests have been updated to ensure coverage of binary and
text types.
@kwoodhouse93
Copy link
Contributor Author

kwoodhouse93 commented Dec 21, 2017

A note on test coverage:

Although coveralls is claiming reduced test coverage based on LOC, test coverage has actually improved when looking at cases covered, as the missed cases are now 2-5 lines long, whereas they were 1 line each in the previous revision.

The missed cases are either unusual data types that are trickier to test (but with behaviour very close to cases we do test), such as JSON or GEOMETRY, or they are field types that are rarely used by the actual MySQL implementations tested with (e.g. fieldTypeVarString is preferred over fieldTypeVarChar). The logic for handling these field types should still be kept, but they are very difficult to test.

Fortunately, these unused field types are handled in exactly the same way as types that we can test, so we can still be fairly confident they will work as expected.

The list of column types we now test with the latest revision is:

  • BIT
  • MEDIUMINT
  • BINARY
  • VARBINARY
  • TINYBLOB
  • TINYTEXT
  • BLOB
  • MEDIUMBLOB
  • MEDIUMTEXT
  • LONGBLOB
  • DATE
  • YEAR

@julienschmidt
Copy link
Member

Thanks for contributing!

@julienschmidt julienschmidt merged commit 9889442 into go-sql-driver:master Jan 10, 2018
@julienschmidt julienschmidt added this to the v1.4 milestone Jan 10, 2018
bLamarche413 pushed a commit to bLamarche413/mysql that referenced this pull request Mar 23, 2018
* Differentiate between BINARY and CHAR

When looking up the database type name, we now check the character set
for the following field types:
 * CHAR
 * VARCHAR
 * BLOB
 * TINYBLOB
 * MEDIUMBLOB
 * LONGBLOB

If the character set is 63 (which is the binary pseudo character set),
we return the binary names, which are (respectively):
 * BINARY
 * VARBINARY
 * BLOB
 * TINYBLOB
 * MEDIUMBLOB
 * LONGBLOB

If any other character set is in use, we return the text names, which
are (again, respectively):
 * CHAR
 * VARCHAR
 * TEXT
 * TINYTEXT
 * MEDIUMTEXT
 * LONGTEXT

To facilitate this, mysqlField has been extended to include a uint8
field for character set, which is read from the appropriate packet.

Column type tests have been updated to ensure coverage of binary and
text types.

* Increase test coverage for column types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants