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

BIT type is defined incorrectly #1597

Open
barkermn01 opened this issue Jul 25, 2022 · 5 comments
Open

BIT type is defined incorrectly #1597

barkermn01 opened this issue Jul 25, 2022 · 5 comments

Comments

@barkermn01
Copy link

barkermn01 commented Jul 25, 2022

So I was looking at trying to fix the implementation of BIT as a Temporary fix till the new RFC system comes in however upon testing and reading, I remember one of the developers saying BIT is treated like TinyInt, and they are not in fact reading the code they have there own type defined in /lib/constants/types.js and this is set as Hex 0x10 however in creating a test case that console.log's the type in both binary_parser and text_parser i found it's providing a type of 253 in a hex that is 0xFD, which is listed as type // aka VARCHAR, VARBINARY,

Could this be that BIT has been changed from the C API-defined version and is now just classed as a VARBINARY with a length of 1?

The test I created to make sure it was using BIT, at first I tried to just use the query SELECT b'1' as val and that seemed to become a blob, so i made sure to test a BIT Column as you can see below.

const createConnection = require('../common.js').createConnection;
const assert = require('assert');

// enabled in initial config, disable in some tets
const c = createConnection({ rowsAsArray: true });
c.query('CREATE TABLE `test_table` (`bit_column` BIT);', (err) => {
    c.query("INSERT INTO `test_table` VALUE(b'1');", (err) => {
        c.query('select b\'1\' as val;', (err, rows) => {
            console.log(rows);
            c.query('DROP TABLE `test_table`;', err => {});
            assert.ifError(err);
            assert.equal(rows[0][0], 1);
        });
    });
})

the Debugging that identified that the Type is incorrect was I added console.log("text_parser type", type); on line 21 of text_parser.js

@sidorares
Copy link
Owner

this is what mysql server returns, I believe if you don't specify charset select b\'1\' as val returns VARCHAR

see https://dev.mysql.com/doc/refman/5.7/en/bit-value-literals.html for more examples

Do you have more context why you think the behaviour you see is incorrect?

@barkermn01
Copy link
Author

Well because as you said if you don't specify charset select b\'1\' as Val returns VARCHAR but it does not it returns a blob, and in the test, I'm explicitly using a BIT column and it still failing, but also I found this because I have a DB table that is production setup so utf8mb4 and BIT type was not returning correctly. instead, I had to move from using the built-in stuff and build a buffered reader to convert the bytes into the correct format. in this case boolean.

@sidorares
Copy link
Owner

hm, you are right. There is no switch case for Types.BIT and it uses default which is "string, unless charset is binary".
I got distracted by select b\'1\' as val' query where server actually returns a string - '\x01'

When you do select * from test_table on your example results are coming with BIT in field type, BINARY as charset and a bunch of data packets with 2 bytes [1, bit value] in the main payload ( 1 for length as payload is still transmitted as "length coded string"

I guess adding following 2 lines should improve the way BIT results are returned:

    case Types.BIT:
      return 'packet.readBuffer(2)[1]';

note that this is potentially a breaking change, but since we are in the process of releasing 3.0.0 might be a good time to add it

All reference this in mysqljs/mysql as currently BIT is returned as 1 length Buffer in that driver as well

@sidorares
Copy link
Owner

actually, 'packet.readBuffer(2)[1]' is not good enough, does not handle NULL ( first byte being 0xfb ), I'll nest a bit some examples with null values inserted

@sidorares
Copy link
Owner

oh, and based on mysqljs/mysql#2559 (comment) BIT can be actually 1 to 8 bytes wide, so my code is completely incorrect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants