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

Add support for BIT type and coerce scalar value to correct types when mysql_server_prepare=0 #53

Merged
merged 2 commits into from
Sep 19, 2016

Conversation

pali
Copy link
Member

@pali pali commented Sep 18, 2016

Without mysql_server_prepare=1 DBD::mysql returns stringified numbers (e.g. "1") for columns of INT type. Second patch fix this problem and coerce scalars to correct perl types.

Both patches adding support for MYSQL_TYPE_BIT. When value has max 32 bits then it is stored as perl unsigned integer. Otherwise as raw string. Address bug: https://rt.cpan.org/Public/Bug/Display.html?id=88006

MySQL BIT type store max 64bits, but perl's UV can handle max 32bits. So
Based on size it is stored to perl scalar as UV or string.

This should fix bug: https://rt.cpan.org/Public/Bug/Display.html?id=88006
…ot used

Currently without mysql_server_prepare all fetched scalars are returned as
strings which totally ignores MySQL column types. With this patch returned
string values are coerced to perl's UV/IV/NV/PV based on column type.

So MySQL value of type INT will be returned as integer scalar, not string
scalar like with mysql_server_prepare=1.
@pali
Copy link
Member Author

pali commented Sep 18, 2016

Looks like travis is broken as it skipped all tests with message:

t/05dbcreate.t ....................... # DBI connect('test','travis',...) failed: Unknown database 'test' at t/05dbcreate.t line 16.
t/05dbcreate.t ....................... skipped: no database connection
t/10connect.t ........................ # DBI connect('test','travis',...) failed: Unknown database 'test' at t/10connect.t line 14.

@mbeijen
Copy link
Contributor

mbeijen commented Sep 19, 2016

Thanks for your pull request. It looks good!

I think I broke Travis and AppVeyor tests myself when I removed creating
test databases here:

138d73f

I'll try look into that later today.

Michiel

Op zondag 18 september 2016 heeft pali [email protected] het
volgende geschreven:

Looks like travis is broken as it skipped all tests with message:

t/05dbcreate.t ....................... # DBI connect('test','travis',...) failed: Unknown database 'test' at t/05dbcreate.t line 16.
t/05dbcreate.t ....................... skipped: no database connection
t/10connect.t ........................ # DBI connect('test','travis',...) failed: Unknown database 'test' at t/10connect.t line 14.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#53 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAoQMDbO8-R07ZprSrHEVWo5n0YYT7BJks5qra7zgaJpZM4KABbm
.

@pali
Copy link
Member Author

pali commented Sep 19, 2016

Looks like t/05dbcreate.t is broken, because in that test is call $dbh->do("CREATE DATABASE IF NOT EXISTS $test_db"). Probably regex for 'remove database from DSN' in that file is incorrect.

@mbeijen
Copy link
Contributor

mbeijen commented Sep 19, 2016

PR welcome :D

On Mon, Sep 19, 2016 at 9:36 AM, pali [email protected] wrote:

Looks like t/05dbcreate.t is broken, because in that test is call $dbh->do("CREATE
DATABASE IF NOT EXISTS $test_db"). Probably regex for 'remove database
from DSN' in that file is incorrect.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#53 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAoQMNxF4JxpuHoM8eZtfJg1q7kNSgiIks5qrjtngaJpZM4KABbm
.

@pali
Copy link
Member Author

pali commented Sep 19, 2016

Easy, PR is here: #54

Btw, BIT conversion code is updated and simplified.

@mbeijen mbeijen merged commit 9e9c912 into perl5-dbi:master Sep 19, 2016
@mbeijen
Copy link
Contributor

mbeijen commented Sep 19, 2016

Many thanks!

@mbeijen
Copy link
Contributor

mbeijen commented Sep 19, 2016

Your fix seems to fail on MySQL 5.7:
https://ci.appveyor.com/project/mbeijen/dbd-mysql-mf7me
Can you look into that?

@pali
Copy link
Member Author

pali commented Sep 19, 2016

That is probably windows-related... Is there any way to test some patch for that windows mysql 5.7 test suite?

@mbeijen
Copy link
Contributor

mbeijen commented Sep 19, 2016

You can make a PR and it will run on AppVeyor.

And I ran the test suite on my Mac with 5.7 and that produces the same
results, so it is not Windows-related.

Op maandag 19 september 2016 heeft pali [email protected] het
volgende geschreven:

That is probably windows-related... Is there any way to test some patch
for that windows mysql 5.7 test suite?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#53 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAoQMGJrwSAkO_O1G5LbKdHbewuQynnhks5qrmKJgaJpZM4KABbm
.

@pali
Copy link
Member Author

pali commented Sep 19, 2016

Now I see where could be problem...

Error message is:
DBD::mysql::db do failed: Data too long for column 'flags' at row 3 at t/rt88006-bit-prepare.t line 34.

Line 34 is:
ok $dbh->do("INSERT INTO dbd_mysql_rt88006_bit_prep (id, flags) VALUES (1, b'10'), (2, b'1'), (3, b'1111111111111111111111111111111111111111')");

And there is:

CREATE TEMPORARY TABLE `dbd_mysql_rt88006_bit_prep` (
`flags` bit(32) NOT NULL

Can you try to change bit(32) to bit(64) so it can store those 40 bits in above INSERT?

@mbeijen
Copy link
Contributor

mbeijen commented Sep 19, 2016

Thanks! Of course that worked 👍

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

Successfully merging this pull request may close these issues.

2 participants