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

add named parameter #43

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

add named parameter #43

wants to merge 4 commits into from

Conversation

voku
Copy link

@voku voku commented Apr 28, 2018

#41

Copy link
Contributor

@clue clue left a comment

Choose a reason for hiding this comment

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

Thank you very much for providing this PR!

I wonder if it makes sense to accept multiple arguments with an array each or maybe limit this to a single argument array à la bindParamsFromArray()? Also, the method definition being public and returning an array looks a bit odd to me. What do you think about this? 👍

e.g. composer -> "could not be downloaded (HTTP/1.1 404 Not Found)
@voku
Copy link
Author

voku commented Apr 28, 2018

Hi, method definition is private and will return only the new parsed sql-query as "string". The feature is now also working with bindParamsFromArray() and bindParams(), tested added.

@Sarke
Copy link

Sarke commented Jul 1, 2019

Hi @voku and @clue, can I ask why this is stagnant?

If you don't mind, I have some thoughts on the suggested implementation.

It doesn't allow multiple uses of a token.

$query = new Query('select * from test where name = :id or id = :id');
$sql = $query->bindParamsFromArray([':id' => 2])->getSql();
$this->assertEquals("select * from test where name = 2 or id = 2", $sql);

This will only replace the first token, not the subsequent ones.

-'select * from test where name = 2 or id = 2'
+'select * from test where name = 2 or id = :id'

It is not string aware.

$query = new Query('select * from test where name = ? or id = :id');
$sql = $query->bindParamsFromArray(["I'm a code sample like self::identify()", ':id' => 2])->getSql();
$this->assertEquals("select * from test where name = 'I'm a code sample self::identify()' or id = 2", $sql);
-'select * from test where name = 'I'm a code sample self::identify()' or id = 2'
+'select * from test where name = 'I\'m a code sample like self:2entify()' or id = :id'

That said, I would like to see this implemented, and could help out.

One thought is to use a SQL lexer like https://packagist.org/packages/phpmyadmin/sql-parser#4.3.2 to tokenize the SQL to make it easier to replace on only the token parts and/or validate the SQL.

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.

3 participants