Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Added support and tests for multiple result sets #88

Merged
merged 17 commits into from
Oct 8, 2022

Conversation

Du-z
Copy link
Collaborator

@Du-z Du-z commented Oct 7, 2022

No description provided.

@Du-z
Copy link
Collaborator Author

Du-z commented Oct 7, 2022

Includes a massive simplification of result handling.

Copy link
Collaborator

@ProphetLamb ProphetLamb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The external API is looking good, but internally the IResponse implementations should not contain pointer arrays. I suggest storing RawResult instead, for the following reasons:

  • Less initial overhead because of Lazy behaviour
  • SoC for Response vs Result
  • While multiple enumeration is possible, the conversion overhead is minimal.
  • No boxing of value types

I suspect creating the OkResults and ErrorResults of a Response at least twice is still faster than n+1 GC allocs and subsequent GC destroys

@Du-z
Copy link
Collaborator Author

Du-z commented Oct 8, 2022

Not quite sure of the implementation details you are asking for. Is this this change what you are looking for 2353e79 .

It is deferring the deserialization of the results for as long as possible, and in some cases skip deserialization all together.

Feel free to tweak and merge if you wish.

@ProphetLamb ProphetLamb merged commit b749267 into master Oct 8, 2022
@ProphetLamb ProphetLamb deleted the multiple_result_sets branch October 8, 2022 21:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants