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

Issue-15363 fix Db->delete bindParam and placeholder not array! #15367

Closed
wants to merge 2 commits into from

Conversation

renatogabrielbr
Copy link
Contributor

I`m pretty sure that has some way to do it better, but i have compile here and worked fine

#15363

db->delete(statement) fail
db->delete(statemente,null) fail
db->delete(statement,null,null)fail

$db->delete(statemente,[],[])
works.

now $db->delete(statement) also works
$db->delete(statement,null,null)

Fix db->delete 

since delete has null as default and are send null to parent execute that only accept  array
Copy link
Member

@niden niden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tidytrax thank you for this contribution.

A couple of changes please. First target the 5.0.0 branch. We do not send anything to master unless we do a full release.

Second, we need some tests for this. There are multiple adapters that this needs to be tested on.

Thanks again

@renatogabrielbr
Copy link
Contributor Author

@tidytrax thank you for this contribution.

A couple of changes please. First target the 5.0.0 branch. We do not send anything to master unless we do a full release.

Second, we need some tests for this. There are multiple adapters that this needs to be tested on.

Thanks again

Hi, yes I did talk to @Jeckerson he did say it for the branch I did target wrong, for the test i will do soon.
and i will improve this code, @Jeckerson has show me the path!

thanks!

I will close it for now, and soon i will pull a new one!

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