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

fix MARS workflow issue when there is an empty result set inside #28

Merged
merged 5 commits into from
Aug 20, 2022

Conversation

ReynBer
Copy link
Collaborator

@ReynBer ReynBer commented Aug 18, 2022

To fix the behaviour on a multiple result set workflow when there is one or more empty result set inside (not the first one).
Currently, as soon as the empty result set is reached, it stops the reading and we have a partial result.

@ReynBer ReynBer changed the title Fix/mars fix MARS workflow issue when there is an empty result set inside Aug 18, 2022
@akhansari
Copy link

Hi Reynald ^^

The dotnet version is in Directory.Build.props file, so as not to change all projects.

@natalie-o-perret
Copy link
Member

natalie-o-perret commented Aug 20, 2022

Hi @ReynBer, just finally catching up with the MR like what we discussed the other day, now that I have been discharged from the hospital, and so far, from what I've skimmed through, this lgtm 👍.

[EDIT] thanks a lot btw (didn't wanna sound ungrateful)

Though, I need to conduct some additional tests (I've spotted something a lil' weird, continuation-wise, just wanna make sure we don't have another type of regression). I have some errands to attend today and I might also need to add an extra change.

I will rebase and merge tonight after the nurse is done changing the bandages, will push a release between tonight and tomorrow evening.

Just one wee tiny bit of nitpicking here, imo, this is not only a MARS issue but a more global continuation problem when fetching the rows (and when the reader is actually stopping), the title can be a tad misleading as MARS echoes a bit too much with SQL Server, but this could happen with any other type of provider that would behave similarly under some circumstances.

Note: @akhansari thanks for the review.

@natalie-o-perret natalie-o-perret merged commit c3057c1 into main Aug 20, 2022
@natalie-o-perret natalie-o-perret deleted the fix/mars branch August 20, 2022 22:40
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.

None yet

3 participants