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

Removed empty queries with nullable relationship. #5

Closed
wants to merge 1 commit into from

Conversation

besanek
Copy link

@besanek besanek commented Jul 14, 2013

Situation: An entity with nullable one reference. If you find only one entity that has this property is set to null, it generates the empty query.

Example

$book = new Book;
$book->author = NULL;
$book->title = "The book written by anonym";
$bookRepository->persist($book);

///

$bookRepository->find($book->id);

Generates :

SELECT `file`.* 
FROM `file` 
WHERE `file`.`id` IN (NULL)

@Tharos
Copy link
Owner

Tharos commented Jul 15, 2013

Thank you very much for this report with included solution.

You are right and I've just fixed this issue in develop branch (I've also added test – that's why I didn't simply merge your code).

@Tharos Tharos closed this Jul 15, 2013
@Majkl578
Copy link

@Tharos: Just a side-note: a more polite solution would be cherry-picking @besanek's commit and editing it so the authorship is retained, additionally if you specify e.g. closes [#5] in commit message, the pull/issue will be automatically closed (and the commit referenced from it). This is how e.g. dg does it when merging some pull requests. :) (Don't take it as an insult, it's just an advice.)

@Tharos
Copy link
Owner

Tharos commented Jul 16, 2013

Thank you for your advice. I didn't know this workflow before and I'll definitely use it next time. :) I'm not experienced in handling pull requests enough. So I really appreciate your effort to teach me how to do that well. :)

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.

3 participants