Skip to content

NH-3951 - support of .All as query result. Test cases, fix and refactoring #556

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

Merged
merged 1 commit into from
Mar 26, 2017

Conversation

fredericDelaporte
Copy link
Member

Implements NH-3951.

This allows calls such as session.Query<Entity>().All(e => someCondition)), instead of having to work-around with !session.Query<Entity>().Any(e => !someCondition)).

Support of the Any case was using some "after-thought" tree inspection, I have simplified it by handling it in the ProcessAny result operator processor. This has required exposing the _isRoot member of IntermediateHqlTree through an IsRoot getter, in order to avoid compromising usages of Any|All as sub-queries.

Of course, all test cases continue to pass. (Excepted the DtcFailuresFixture.Can_roll_back_transaction which has an unrelated TearDown issue (not closing its connection), which occurs on my setup without my changes too.)

I would like to be able to go on fixing NH-3850 with changes done by this NH-3951.

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Feb 22, 2017

As for 555 pull, I do not understand how my changes may relate to the Oracle 32 failing cases.

(Edit: now I have seen the message on nhibernate development about troubles with DTC, causing those failures with Oracle 32, unrelated to pull request changes.)

@fredericDelaporte fredericDelaporte changed the title NH-3951: test cases, fix and refactoring NH-3951: support for .All as query resuly. Test cases, fix and refactoring Feb 25, 2017
@fredericDelaporte fredericDelaporte changed the title NH-3951: support for .All as query resuly. Test cases, fix and refactoring NH-3951: support of .All as query resuly. Test cases, fix and refactoring Feb 25, 2017
@fredericDelaporte fredericDelaporte changed the title NH-3951: support of .All as query resuly. Test cases, fix and refactoring NH-3951: support of .All as query result. Test cases, fix and refactoring Feb 25, 2017
@@ -53,11 +61,6 @@ public IntermediateHqlTree(bool root)

public ExpressionToHqlTranslationResults GetTranslation()
{
if (_isRoot)
{
DetectOuterExists();
Copy link
Member Author

Choose a reason for hiding this comment

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

Now handled in ProcessAny result operator.

tree.AddWhereClause(tree.TreeBuilder.BooleanNot(
HqlGeneratorExpressionTreeVisitor.Visit(resultOperator.Predicate, queryModelVisitor.VisitorParameters).
ToBooleanExpression()));
public class ProcessAll : IResultOperatorProcessor<AllResultOperator>
Copy link
Member Author

Choose a reason for hiding this comment

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

Was formatted with spaces, got formatted with tabs.

@hazzik
Copy link
Member

hazzik commented Feb 28, 2017

Looks legit

Copy link
Member

@gliljas gliljas left a comment

Choose a reason for hiding this comment

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

Looks great!

@fredericDelaporte fredericDelaporte changed the title NH-3951: support of .All as query result. Test cases, fix and refactoring NH-3951 - support of .All as query result. Test cases, fix and refactoring Mar 18, 2017
…toring .Any as final result for avoiding HQL tree inspection.
@fredericDelaporte
Copy link
Member Author

Rebased and squashed.

@gliljas gliljas merged commit bad8b20 into nhibernate:master Mar 26, 2017
@fredericDelaporte fredericDelaporte deleted the NH-3951 branch March 26, 2017 11:17
@fredericDelaporte fredericDelaporte removed the request for review from oskarb March 26, 2017 11:20
@fredericDelaporte fredericDelaporte added this to the 5.0.0 milestone Mar 26, 2017
@hazzik hazzik added the r: Fixed label Aug 3, 2017
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