-
Notifications
You must be signed in to change notification settings - Fork 934
Throw for DML on filter #2020
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
Throw for DML on filter #2020
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -282,6 +282,9 @@ public virtual void SetResultTransformerAndAdditionalCriteria(IQuery query, NhLi | |
|
||
public int ExecuteDml<T>(QueryMode queryMode, Expression expression) | ||
{ | ||
if (Collection != null) | ||
throw new NotSupportedException("DML operations are not supported for filters."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As potentially it could be implemented I think NotSupported/NotImplemented exceptions are more appropriate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NotSupported is fine |
||
|
||
var nhLinqExpression = new NhLinqDmlExpression<T>(queryMode, expression, Session.Factory); | ||
|
||
var query = Session.CreateQuery(nhLinqExpression); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maca88 I've just noticed that async versions of DML extension methods are not used at all in async tests. Is it Async Generator bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's a bug and I've created an issue for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Published a new release
0.13.2
that has the bug fixed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think we should upgrade the async generator on 5.2x (and even less on previous branches). So we will have to take care of regenerating async files as part of the merge to master I think, in order to take into account the fix now available in master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thoughts