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

skip take & select fields #184

Merged
merged 15 commits into from
Sep 25, 2020

Conversation

nzdev
Copy link
Contributor

@nzdev nzdev commented Sep 15, 2020

For paging and better performance there's a new SkipTake method available on search results. Prior to this if using the built in Skip method for paging it would still allocate search results for the items being skipped. With SkipTake this is no longer the case.


Original issue:

What:
Skip and Take implementation that does not collect skipped documents. Select only required fields

Why?
Saves on allocating skipped documents. Separate implementation to existing skip as this one can only be called once, so if you need to get another page, you need to run the query again, unlike using maxresults.

when calling ExecuteWithSkip(int pageSize, int numberOfResultsToSkip), then when calling .Skip(int numberOfResultsToSkip) use .Skip(0) as the results have already been skipped.

Copy link
Owner

@Shazwazza Shazwazza left a comment

Choose a reason for hiding this comment

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

Great stuff, I didn't realize you could skip over the docs. I have left some notes about breaking changes again and it seems like the PR for field inclusions has leaked into this one if that can be fixed up?

src/Examine/Search/IQueryExecutor.cs Show resolved Hide resolved
@nzdev
Copy link
Contributor Author

nzdev commented Sep 23, 2020

@Shazwazza updates made

@Shazwazza Shazwazza changed the base branch from master to feature/skip-take September 25, 2020 04:26
@Shazwazza Shazwazza merged commit afdf1c0 into Shazwazza:feature/skip-take Sep 25, 2020
@Shazwazza
Copy link
Owner

Thanks! Have merged to a temporary branch for now as I think there's some mods I'd like to make before merging to master. Cheers!

@Shazwazza Shazwazza added this to the 1.1.0 milestone Sep 25, 2020
@nzdev
Copy link
Contributor Author

nzdev commented Sep 25, 2020

Thanks @Shazwazza I'll update the umbraco pr once this is released.

@Shazwazza
Copy link
Owner

@nzdev if you want to have a look, i've refactored in this rev: 36f4a5e Basically moving SkipTake to where Skip also was so the fluent API remains consistent. This is all working now and allows re-execution too. The only pretty ugly part is how the lazy execution of the TotalItemCount works, but to avoid breaking changes this is fine for now. Let me know what you think. I've also updated your test so probably easiest to just look at that one.

@nzdev
Copy link
Contributor Author

nzdev commented Sep 28, 2020

Seems OK, though Execute() no longer actually executes the query.

@Shazwazza
Copy link
Owner

Execute() no longer actually executes the query

Yep that's true. That's also how it works in ExamineX as well since the execution is lazy. I don't think this is an issue but perhaps in the next netcore version we can re-think a bit of how this could/should work.

@nzdev
Copy link
Contributor Author

nzdev commented Feb 10, 2021

Any news on when this will be merged to master?

@Shazwazza
Copy link
Owner

It's already merged and pending release https://github.com/Shazwazza/Examine/milestone/25?closed=1

was just waiting on some feedback from the pre-release. I'll get this out next week.

@nzdev
Copy link
Contributor Author

nzdev commented Feb 11, 2021

@Shazwazza Thanks, sorry I hadn't seen that.

@nzdev
Copy link
Contributor Author

nzdev commented Feb 16, 2021

The skiptake methods seem to not be present in v1.1.0

@Shazwazza
Copy link
Owner

🤦 🤦‍♂️ 🤦 🤦‍♂️ 🤦 🤦‍♂️ 🤦 🤦‍♂️

Sorry, i'll make another release, that is just stupid of me

@Shazwazza Shazwazza modified the milestones: 1.1.0, 1.2.0 Feb 16, 2021
@Shazwazza
Copy link
Owner

@nzdev there's a build here https://ci.appveyor.com/project/Shandem/examine/build/artifacts which is a 1.2.0 alpha. If you are able to test this release with your current setup for a bit, i'll get this one released by the end of the week or early next week.

@Shazwazza
Copy link
Owner

The nuget feed for that is here https://ci.appveyor.com/nuget/examine-f73l6qv0oqfh

@nzdev nzdev deleted the v1/skip-take-select branch February 1, 2023 08:19
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.

2 participants