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 max parameters Count #139

Merged
merged 4 commits into from
Nov 22, 2024

Conversation

Red1408192
Copy link

@Red1408192 Red1408192 commented Nov 25, 2022

I suggest a fix in this section here
I recently got stucked by the really unlucky scenario where the parameters count can exceed the number allowed by sql server

Related issue: #138

i have also suggested a slight performance improvement and reduced the parameters cap by 10, just for safety

@Red1408192 Red1408192 changed the title Fix max parameter Fix max parameters Count Nov 25, 2022
@Red1408192
Copy link
Author

Red1408192 commented Nov 25, 2022

fixed, the tests require the exact same order for the parameters to pass, so i had to recalculate before hand the update expressions. tecnically it wouldn't have been a bug, but instead of updating all the unit tests i prefered to do like so

@artiomchi
Copy link
Owner

Hey @Red1408192, apologies for only picking this up now

I went through the code, and I realised that you never actually changed the order of arguments being inserted, so the updated code could be.. simplified.. back to the original way of indexing arguments.

You're absolutely right that I completely forgot about taking into account the update expression and the condition constants.. I've further optimised it to calculate them only once into the singleEntityArguments variable, since that was the purpose of it :)

@artiomchi artiomchi force-pushed the fix/maxArgumentCount branch from 1fc382b to c99d5ac Compare November 22, 2024 21:08
@artiomchi
Copy link
Owner

Thank you for the contribution - going to merge it, and release it in the next update 👍

@artiomchi artiomchi merged commit c2b96fc into artiomchi:develop Nov 22, 2024
2 checks passed
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.

2 participants