Add test for large BLOB with parameter #178
Conversation
illuusio
force-pushed
the
add-large-blob-test
branch
from
2bf5c9a
to
79be194
Compare
|
As noted in #179 the correct application behavior is to bind the parameter as binary when inserting it. |
illuusio
force-pushed
the
add-large-blob-test
branch
from
546c572
to
5f36cfe
Compare
|
Ok now using correct |
illuusio
force-pushed
the
add-large-blob-test
branch
2 times, most recently
from
988c6c1
to
cec23c1
Compare
|
Hello! I would like to ask what you motivated to write this test without any code change? Have you triggered some issue in the driver code? Or is there some known bug related to LONGBLOB type? About test itself, I do not like two things: Usage of rand() and dropping table at the beginning of the test. Test should be deterministic, not random because it would be harder to debug it if it fails only sometimes. And instead of dropping table at the beginning of the test you can use temporary table which is visible only for the current $dbh connection, and is automatically dropped after closing it. Anyway, repository now contains new Github Actions CI which run tests from the every pull request against lot of different client and server versions. So after rebasing your PR on top of the master branch, github runs these new tests automatically. |
|
Btw, when you are adding usage of a new module in test as hard dependency then it is needed to specify that module in TEST_REQUIRES section in Makefile.PL. |
|
Should this be closed altogether as this was just me who was thinking that it could be nice to have this kind of test. As I faced this when testing Perl CHI DBI driver and it didn't work with MariaDB but it worked just fine with MySQL, Postgres and SQLite. As said this just RTFM like people like who don't read Pod/Man pages. |
|
I'm not against such test. Just fix issues in the test which I mentioned above. |
illuusio
force-pushed
the
add-large-blob-test
branch
from
cec23c1
to
190110b
Compare
illuusio
force-pushed
the
add-large-blob-test
branch
from
190110b
to
b85a201
Compare
|
Ok i've updated this test but have to solve |
|
@illuusio could you fix test to pass CI? |
illuusio
force-pushed
the
add-large-blob-test
branch
from
b85a201
to
f8179a6
Compare
|
Sorry this dropped out of my scope for other things but let's see if 512 KB is small enough to make old MySQL and MariaDB happy. I've also rebased branch. |
Add test for large binary using MariaDB LARGEBLOB column type with size of 1 M test binary. After inserting make sure it can be fetched from database and it's not altered. As mentioned in Pod documentation test uses bind_param as normal excute will not handle binary blobs correctly
illuusio
force-pushed
the
add-large-blob-test
branch
from
f8179a6
to
f141379
Compare
|
@choroba any other comments? |
|
No more comments 🙂 |
Add test for large binary BLOB inserting with prepared statement and large Gzip binary as parameter.