Skip to content

Fix bug where a non-sproc command comes before a sproc command#29680

Merged
roji merged 2 commits into
dotnet:mainfrom
roji:SprocMixup
Dec 1, 2022
Merged

Fix bug where a non-sproc command comes before a sproc command#29680
roji merged 2 commits into
dotnet:mainfrom
roji:SprocMixup

Conversation

@roji

@roji roji commented Nov 25, 2022

Copy link
Copy Markdown
Member

Fixes #29643

@roji roji requested a review from AndriySvyryd November 30, 2022 14:59

if (columnModification.UseCurrentValueParameter)
{
parameterCount++;

@AndriySvyryd AndriySvyryd Nov 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would also make sense to do this when building the modification command and expose the count for both types of commands though a single property

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I thought about this, and didn't do this mainly since we want to backport the fix to 7.0, and should avoid new public APIs. I can do this in a separate PR after this is merged.

I'm not sure I'd eagerly calculate this though (during the building of the command) - it's currently only needed when output parameters are present, which is a rare sproc-only case.

@roji roji merged commit 35e9e5a into dotnet:main Dec 1, 2022
@roji roji deleted the SprocMixup branch December 1, 2022 07:34
roji added a commit to roji/efcore that referenced this pull request Dec 1, 2022
wtgodbe pushed a commit that referenced this pull request Jan 4, 2023
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.

Mixing sproc and non-sproc commands in the same batch may fail with a NullReferenceException

2 participants