Skip to content

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

Merged
sergeyklay merged 3 commits into
Codeception:masterfrom
W0rma:fix-parameter-type-for-null
Jan 12, 2024
Merged

Fix Db::executeQuery() for null parameter#63
sergeyklay merged 3 commits into
Codeception:masterfrom
W0rma:fix-parameter-type-for-null

Conversation

@W0rma

@W0rma W0rma commented Dec 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 mentioned this pull request Dec 1, 2023
Merged
@W0rma W0rma changed the title Fix parameter type for null Fix Db::executeQuery() for null parameter Dec 1, 2023
@pascal08

pascal08 commented Dec 5, 2023 •
edited
Loading

Copy link
Copy Markdown

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

@tecbird

tecbird commented Dec 6, 2023

Copy link
Copy Markdown

same here, please merge

@troy-rudolph

troy-rudolph commented Dec 7, 2023

Copy link
Copy Markdown

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

shannon-programming commented Dec 11, 2023 •
edited
Loading

Copy link
Copy Markdown

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

MattiaPeiretti commented Dec 11, 2023

Copy link
Copy Markdown

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.

This was referenced Dec 13, 2023
Closed
Closed
guraurobi
guraurobi suggested changes Dec 13, 2023
View reviewed changes
Comment thread src/Codeception/Lib/Driver/Db.php
Comment thread composer.json
@laoneo

laoneo commented Dec 21, 2023

Copy link
Copy Markdown

Please merge

@MattiaPeiretti

MattiaPeiretti commented Jan 3, 2024 •
edited
Loading

Copy link
Copy Markdown

33 days old.

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

@eerison

eerison commented Jan 3, 2024 •
edited
Loading

Copy link
Copy Markdown

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

Daredzik commented Jan 8, 2024

Copy link
Copy Markdown

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

Daredzik commented Jan 10, 2024

Copy link
Copy Markdown

@Naktibalda or @sergeyklay can you check this one?

@eerison

eerison commented Jan 10, 2024 •
edited
Loading

Copy link
Copy Markdown

@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

Daredzik commented Jan 10, 2024

Copy link
Copy Markdown

@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

MattiaPeiretti commented Jan 11, 2024

Copy link
Copy Markdown

42 days old.

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

@sergeyklay sergeyklay merged commit 5666795 into Codeception:master Jan 12, 2024
@sergeyklay

sergeyklay commented Jan 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@MattiaPeiretti

MattiaPeiretti commented Jan 12, 2024

Copy link
Copy Markdown

@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

Reviewers

@Arhell Arhell Arhell approved these changes

+5 more reviewers

@Daredzik Daredzik Daredzik left review comments

@eerison eerison eerison left review comments

@troy-rudolph troy-rudolph troy-rudolph approved these changes

@guraurobi guraurobi guraurobi requested changes

@sergeyklay sergeyklay sergeyklay approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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