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

Allow query parameters starting with an underscore #717

Merged
merged 2 commits into from
Jul 29, 2013

Conversation

hackedd
Copy link
Contributor

@hackedd hackedd commented Jul 4, 2013

Using a query parameter of which the name starts with an underscore results in a QueryException. For example:

$q = $em->createQueryBuilder()
    ->select("u")
    ->from("User", "u")
    ->where("u.username = :_name")
    ->setParameter("_name", "bar")
    ->getQuery();

Results in:

Invalid parameter format, : given, but :<name> or ?<num> expected.

This happens because of a bug in the Lexer, which recognizes :_name as two tokens (the : as the start of a input parameter, _name as an identifier). The attached patch changes the regular expression for input parameters to allow identifiers starting with a letter or underscore.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-2544

We use Jira to track the state of pull requests and the versions they got
included in.

@Ocramius
Copy link
Member

Ocramius commented Jul 4, 2013

@hackedd please add tests that would fail without this patch

@stof
Copy link
Member

stof commented Jul 5, 2013

The test should be in the unit tests of the lexer, not in the functional tests

guilhermeblanco added a commit that referenced this pull request Jul 29, 2013
Allow query parameters starting with an underscore
@guilhermeblanco guilhermeblanco merged commit a53fe14 into doctrine:master Jul 29, 2013
@hackedd hackedd deleted the patch-1 branch July 30, 2013 08:54
@beberlei
Copy link
Member

@guilhermeblanco do we actually want this as valid parameter name? I am not sure

@asm89
Copy link
Member

asm89 commented Aug 11, 2013

I was asking myself the same question. Unless someone has good arguments my
currently thought are -1 for this change.

On Sat, Aug 10, 2013 at 6:13 PM, Benjamin Eberlei
[email protected]:

@guilhermeblanco https://github.com/guilhermeblanco do we actually want
this as valid parameter name? I am not sure


Reply to this email directly or view it on GitHubhttps://github.com//pull/717#issuecomment-22442449
.

@guilhermeblanco
Copy link
Member

@beberlei @asm89 we do. For identifiers we should support same thing PHP support for variable declaration. =)

@beberlei
Copy link
Member

@guilhermeblanco why "should"? This parameters don't match to php variable names anywhere, i don't see the relation. This only adds another way for something that already works when using sane identifier naming rules

@guilhermeblanco
Copy link
Member

@beberlei some people (me included) like to use the parameter name as the same as the variable name. It's sane (and also seems to be supported by HQL) to support same identifiers as variables.

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.

7 participants