Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Thread-Safe SqlGenerator implementation #309

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

xamele0n
Copy link

@xamele0n xamele0n commented Oct 6, 2022

Default SqlGenerator has properties
IList<IColumn> AllColumns { get; } IList<Table> MappedTables { get; }
that filled and modified in different methods. This data shared across methods inside SqlGenerator in non-threadsafe manner and it can be changed by differrent (parallel) thread.
Whole sql statement must be composed by it`s own SqlGenerator

@xamele0n
Copy link
Author

xamele0n commented Oct 6, 2022

Fix #303 #298 #274
Requesting @valfrid-ly for review

@skovsende
Copy link

@xamele0n Do you happen to have created a nuget package with this fix that you can share? :)

@skovsende
Copy link

Nm. I made one myself: https://www.nuget.org/packages/Swush.DapperExtensions/1.7.1-hotfix-concurrency

It's basically just the code from this PR. Do note that it only supports .net 6 and 7.

@skovsende
Copy link

Anyway - deployed the fix on production - and got the same error as described in #298 almost immediately. So this does not fix #298 - unfortunately!

@xamele0n
Copy link
Author

@skovsende - well.. i did`t noticed that fields are used outside forming sql. I did rewrite implementation according your case. You can try this.

@skovsende
Copy link

skovsende commented Apr 14, 2023

Great job! It fixes the code I posted in #298. I decided to get rid on our dependency on DapperExtensions as we weren't really using it for much, but for everyone else I have uploaded a package based on this PR here

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants