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

Nested Cosmos Db compatibility #352

Merged
merged 6 commits into from
Mar 18, 2020

Conversation

countincognito
Copy link
Contributor

There seems to be an issue when using nested queries (i.e. >1 level deep), such as Sum() and Any(), which causes the query provider in the Azure Cosmos SDK to generate invalid SQL. I was able to fix it by overriding empty parameter expression names. Apologies if I have missed anything.

@countincognito countincognito requested a review from StefH February 25, 2020 17:08
@codecov
Copy link

codecov bot commented Feb 25, 2020

Codecov Report

Merging #352 into master will increase coverage by 0.33%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
+ Coverage   88.13%   88.46%   +0.33%     
==========================================
  Files          46       46              
  Lines        4500     4509       +9     
==========================================
+ Hits         3966     3989      +23     
+ Misses        534      520      -14
Impacted Files Coverage Δ
...ystem.Linq.Dynamic.Core/Parser/ExpressionParser.cs 87.82% <100%> (+0.92%) ⬆️
...tem.Linq.Dynamic.Core/ParameterExpressionHelper.cs 100% <100%> (ø) ⬆️
src/System.Linq.Dynamic.Core/ParsingConfig.cs 100% <100%> (ø) ⬆️
...em.Linq.Dynamic.Core/DynamicQueryableExtensions.cs 96.52% <100%> (ø) ⬆️
...ystem.Linq.Dynamic.Core/DynamicExpressionParser.cs 85.24% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 849952f...b7949ce. Read the comment docs.

@countincognito countincognito changed the title Cosmos db compatibility Nested Cosmos Db compatibility Feb 26, 2020
Copy link
Collaborator

@StefH StefH 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 for this PR, I never thought this library could be used for CosmosDB.

I'm reviewing the code, some comments are added, probably need some more time to check other items.

@countincognito
Copy link
Contributor Author

Thank you for this PR, I never thought this library could be used for CosmosDB.

My pleasure - such a great library, I only hope my contribution is up to standard. It's been phenomenally useful for our project with Cosmos Db (we use it in conjunction with the Cosmonaut library). This issue with the nested queries was the final hurdle for us to allow for more complicated queries.

@countincognito countincognito requested a review from StefH February 26, 2020 20:07
@countincognito
Copy link
Contributor Author

@StefH Any further changes required?

@StefH
Copy link
Collaborator

StefH commented Feb 29, 2020

@countincognito Thanks for this PR and your reworked code.

I'm doubting if I should just add this to the next version 1.0.21 / 1.0.22 or that I should use a higher version like 1.1.0

@countincognito
Copy link
Contributor Author

@countincognito Thanks for this PR and your reworked code.

I'm doubting if I should just add this to the next version 1.0.21 / 1.0.22 or that I should use a higher version like 1.1.0

Thanks for approving it. Glad the changes will be of use.

@StefH StefH merged commit 553d0a3 into zzzprojects:master Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants