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

Fix Db::executeQuery() for null parameter #63

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

W0rma
Copy link
Contributor

@W0rma W0rma commented Dec 1, 2023

Seems like #48 broke the following code

$I->seeInDatabase('some_table', ['some_column => null]);

After updating codeception/module-db from 3.1.0 to 3.1.1 I get the following error:

[TypeError] Codeception\Lib\Driver\Db::isBinary(): Argument #1 ($string) must be of type string, null given, called in /var/www/html/vendor/codeception/module-db/src/Codeception/Lib/Driver/Db.php on line 297

This PR should fix that by using PDO::PARAM_NULL if null was provided in the criteria.

Fixes #64

@W0rma W0rma changed the title Fix parameter type for null Fix Db::executeQuery() for null parameter Dec 1, 2023
@pascal08
Copy link

pascal08 commented Dec 5, 2023

Experiencing the same problem. Downgrading to v3.0.1 helps, but introduces another problem (that is fixed in v3.1.0).

@tecbird
Copy link

tecbird commented Dec 6, 2023

same here, please merge

@troy-rudolph
Copy link

We have the same problem. #48 causes many of our tests to fail as show below. Please merge and ship this fix.

00:38:41  �[37;41;1m  [TypeError] Codeception\Lib\Driver\Db::isBinary(): Argument #1 ($string) must be of type string, null given, called in /web/manager-jenkins/jenkins-Manager-PR-17842-5/www/vendor/codeception/module-db/src/Codeception/Lib/Driver/Db.php on line 297  �[39;49;22m
00:38:41  �[37;41;1m                                                                                                                                                                                                                                                         �[39;49;22m
00:38:41  �[33m
00:38:41  Scenario Steps:
00:38:41  �[39m
00:38:41   6. $I->updateInDatabase("Tests",{"Publis...},{"TestID...}) at �[32mtests/api/observations/ObservationsFiltersCest.php:222�[39m
00:38:41   5. // I am going to update the Status for getting archived and abandoned Observations (in Tests table)
00:38:41   4. $I->grabFromDatabase("Tests","TestID",{"CourseID":40}) at �[32mtests/api/observations/ObservationsFiltersCest.php:215�[39m
00:38:41   3. $I->grabFromDatabase("Tests","TestID",{"CourseID":38}) at �[32mtests/api/observations/ObservationsFiltersCest.php:210�[39m
00:38:41   2. $I->useDataGeneratorTool({"actions":[{"action":"tun...}) at �[32mtests/api/observations/ObservationsFiltersCest.php:194�[39m
00:38:41   1. $I->getMicroStamp() at �[32mtests/api/observations/ObservationsFiltersCest.php:17�[39m
00:38:41  
00:38:41  #1  Codeception\Module\Db->updateInDatabase
00:38:41  #2  /web/manager-jenkins/jenkins-Manager-PR-17842-5/www/tests/codeception2/tests/_support/_generated/ApiTesterActions.php:4686
00:38:41  #3  /web/manager-jenkins/jenkins-Manager-PR-17842-5/www/tests/codeception2/tests/api/observations/ObservationsFiltersCest.php:222
00:38:41  #4  SM\ObservationsFiltersCest->_before
00:38:41  #5  /web/manager-jenkins/jenkins-Manager-PR-17842-5/www/vendor/bin/codecept:119
00:38:41  �[33mArtifacts:�[39m

@shannon-programming
Copy link

shannon-programming commented Dec 11, 2023

Experiencing the same issue! Please merge this fix!

[TypeError] Codeception\Lib\Driver\Db::isBinary(): Argument #1 ($string) must be of type string, Ramsey\Uuid\Lazy\LazyUuidFromString given, called in /app/vendor/codeception/module-db/src/Codeception/Lib/Driver/Db.php on line 297

@MattiaPeiretti
Copy link

Can someone please merge @W0rma's PR? Real companies are relying on this and the fact that a fix is present for 5 days and it's just waiting to be merged is quite sad.

src/Codeception/Lib/Driver/Db.php Show resolved Hide resolved
composer.json Show resolved Hide resolved
@laoneo
Copy link

laoneo commented Dec 21, 2023

Please merge

@MattiaPeiretti
Copy link

MattiaPeiretti commented Jan 3, 2024

33 days old.

Still waiting. I believe in you, you can do it!

@eerison
Copy link

eerison commented Jan 3, 2024

33 days old.


Still waiting. I believe in you, you can do it!

the quickly solution is, @troy-rudolph @W0rma try to apply @guraurobi suggestions, otherwise it will be in discussion for a long time, because it still have open discussions, and they aren't hard to solve.

@Daredzik
Copy link

Daredzik commented Jan 8, 2024

Merge this, suggestion to use mb_detect_encoding is wrong:
[TypeError] mb_detect_encoding(): Argument #1 ($string) must be of type string, null given
is_string do the trick here

@Daredzik
Copy link

@Naktibalda or @sergeyklay can you check this one?

@eerison
Copy link

eerison commented Jan 10, 2024

@Naktibalda or @sergeyklay can you check this one?

Just to point out, this issue was introduced on 3.1.1, you can back one version and wait until this PR be merged.

add it in your composer.json, and after it pass, you just need to remove it from conflict.

  "conflict": {
    "codeception/module-db": ">3.1.0"
  },

@Daredzik
Copy link

@Naktibalda or @sergeyklay can you check this one?

Just to point out, this issue was introduced on 3.1.1, you can back one version and wait until this PR be merged.

add it in your composer.json, and after it pass, you just need to remove it from conflict.

  "conflict": {
    "codeception/module-db": ">3.1.0"
  },

I know, but ironically, in version 3.1.1, there is a fix for the UTF-8 bug #47, which unfortunately affects me too.

@MattiaPeiretti
Copy link

42 days old.

Still waiting. I believe in you, you can do it!

@sergeyklay sergeyklay merged commit 5666795 into Codeception:master Jan 12, 2024
3 checks passed
@sergeyklay
Copy link
Contributor

sergeyklay commented Jan 12, 2024

Thank you for the patch and I'm sorry for the delay.

@MattiaPeiretti
Copy link

@W0rma W0rma deleted the fix-parameter-type-for-null branch January 12, 2024 10:24
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.

Breaking change in 3.1.1: type error in call to isBinary()