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

Updated implementation of query named parameters #169

Open
wants to merge 2 commits into
base: 0.7.x
Choose a base branch
from

Conversation

Divi
Copy link

@Divi Divi commented Jan 15, 2023

Hello!

I saw a previous pull request about the named parameters. Since there is no activity for 3 years now, I took the liberty to make my own implementation and update the unit tests.

It allows:

$connection->query('SELECT * FROM my_table WHERE foo = :param', ['param' => 'bar']);
$connection->query('SELECT * FROM my_table WHERE foo = :param', [':param' => 'bar']);
$connection->query('SELECT * FROM my_table WHERE foo = :param AND param2 = ?', ['param' => 'bar', 'param2Value']);

Of course, all the previous ways of querying still work.
I can update the README if needed.

Copy link
Contributor

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

Hey @Divi, thanks for opening this pull request, always happy about contributions 👍 👍

I really like the quality of this PR, just left a single remark below.

tests/Io/QueryTest.php Outdated Show resolved Hide resolved
@Divi Divi requested a review from SimonFrings January 21, 2023 00:24
Copy link
Contributor

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for your work on this 👍

@clue
Copy link
Contributor

clue commented Feb 7, 2023

@Divi Thanks for looking into this, would love to get this in!

The additional tests are much appreciated, definitely helps to get some certainty with this feature. It looks like some cases aren't covered right now, can you add some additional tests to ensure this works properly when using the same placeholder name multiple times (SELECT * FROM user WHERE name = :name OR email = :name) and also what happens if the placeholder value contains strings that look like placeholders (['name' => 'is :this a placeholder?']).

@Divi
Copy link
Author

Divi commented Mar 13, 2023

Just to let you know, I've not forgotten you, I just don't have enough time for now but I keep in mind your request :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants