Fix | Pass Empty List for SqlDataRecord (#2971) #3518

sharafabacery · 2025-07-26T10:08:44Z

Description

Important topics to cover include:
Enable Passing Empty List of type SqlDataRecord and make it functionality like when value is null

Description of the functionality.
I remove exception from SqlParameter.cs and in SqlCommand I figure where it if parameter is null what happen and adding small condition

Issues

Fixes issue #2971

Testing

I create database and delete it when test finish to make test work and
first test when list is empty and second one when it null and it works correctly

and this photo captured from my sql server profiler to prove that it sends default

benrr101

Thanks for taking a stab at working on this issue. It needs a lot of work though... Please take a look at the comments and let me know your thoughts.

benrr101 · 2025-07-30T17:10:38Z

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

                        }

                        if (!_pendingCancel)
-                        { 


I don't understand this change at all, especially since they seem to be completely unrelated to the focus of the PR. We don't want to be removing braces for if statements, and in this particular instance it completely upends the logic of the method.

benrr101 · 2025-07-30T17:11:31Z

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

                    asyncWrite,
                    isRetry,
                    methodName);
-


This doesn't really help readability.

benrr101 · 2025-07-30T17:12:17Z

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

                                        {
                                            completion.TrySetResult(retryTask.Result);
                                        }
-                                    }, 


This change also makes no sense, and I don't see how it compiles at all. Why was this change made?

benrr101 · 2025-07-30T17:13:14Z

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

        }

        /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/ExecuteXmlReaderAsync[@name="default"]/*'/>
-        public Task<XmlReader> ExecuteXmlReaderAsync() => 


This change seems completely random, unrelated to the focus of the PR, and likely doesn't compile.

benrr101 · 2025-07-30T17:14:02Z

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

                task: out unused,
                usedCache: out _,
                method: method);
-


Again, removing this line doesn't really aid readability and is unrelated to the PR focus.

All changes above could be because I enable formatting document after saving
@benrr101

benrr101 · 2025-07-30T17:17:29Z

src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlCommandTest.cs

            }
        }
+
+        #region fixing "Change SqlParameter to allow empty IEnumerable<SqlDataRecord>? #2971"


I'm not sure what to do about these tests. I think someone with a bit more experience in the test area could provide more concrete feedback. @mdaigle ?

All changes above could be because I enable formatting document after saving

If there any test need to be done i will do it?

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

edwardneal · 2025-07-30T21:10:27Z

Without wanting to step on @benrr101's toes, does this address the issue and support IEnumerable<SqlDataRecord>?

I've written a simple test - gist for it is here. This test returns an enumerator which isn't backed by an ICollection (and can't be - it's an iterator method.) The test as-is fails with the error:

The incoming tabular data stream (TDS) remote procedure call (RPC) protocol stream is incorrect. Table-valued parameter 1 ("@tvp"), row 0, column 0: Data type 0xF3 (user-defined table type) is null and not set to default. A null table-valued parameter is required to be sent as a default parameter.

One possible approach which might allow iterator methods to work is here. It's enough to prove the principle and hopefully it helps a little, but it's definitely not complete. For a quick overview:

We add an explicit Empty flag to the structure which holds the peekahead information, indicating that the enumerator is empty.
I took this slightly further and have a shared EmptyPeekahead instance so that we're not creating a large number of objects.
SqlParameter.GetActualFieldsAndProperties is the only method which populates this peekahead structure. Rather than throwing SQL.IEnumerableOfSqlDataRecordHasNoRows, we set the peekahead to EmptyPeekahead.
TdsParser.WriteSmiParameter is the only method which calls GetActualFieldsAndProperties and does anything with the peekahead information. It also has a sendDefault parameter, so we make sure that if the Empty flag on the peakahead is set, sendDefault is also forced to true. My test started to pass.
Since we no longer used the SQL.IEnumerableOfSqlDataRecordHasNoRows method, I deleted it.
I added the test in the gist to prove that this works. It passes on .NET Core.

I've not ported this over to the .NET Framework version. I've also only tested this in one specific situation - it's worth testing to prove this behaves properly with multiple table-valued parameters, empty lists, empty arrays and any other situations which come to mind. I'm sure there'll be some feedback on the way the test is written too - it's just to demonstrate the existing code which eliminates the need to create and drop databases manually.

sharafabacery · 2025-07-30T21:39:59Z

@edwardneal so impressive what you do,IF I need to still on this issue i need to do? what i need to learn? to complete this issue?

edwardneal · 2025-08-14T21:48:04Z

Sorry for the delayed response. I think we're dealing with the problem in nearly the same place - the difference here is that your approach sets the options variable correctly in the first instance (based on the type of the parameter) while my approach changes the way that TdsParser responds to that variable a little later in the process (based on whether the IEnumerable has a value.)

I think the EmptyDynamicIEnumerableShouldPassDefault test will prove whether or not #2971 has been fixed (although benrr101 or mdaigle might have some extra feedback.) I'd suggest reverting the changes to SqlCommandTest.cs and using EmptyDynamicIEnumerableShouldPassDefault as your test case to prove whether or not the issue is fixed.

Ben's already pointed out a few of the unrelated changes to SqlCommand.cs, those need to be reverted. Once that's done (and the PR only contains the needed changes) then you can find a working method to fix the problem. After you've found that method, just make sure the two SqlCommand code files (one in netcore and one in netfx) have been changed in the same way.

This reverts commit 1d6ee75.

sharafabacery · 2025-08-25T20:04:39Z

@edwardneal thanks for help,curently I working on it by solution based on my previous one if your testcase fail i will try yours beacuse we try soolve the same problem and each of us look on it some respective, but whenI fetch orgin and run testcases and it build the package it responed with errors
Metadata file 'source\repos\SqlClient\artifacts\Project\bin\Windows_NT\Debug.AnyCPU\Microsoft.Data.SqlClient\netcore\net8.0\Microsoft.Data.SqlClient.dll' could not be found
and there is some syntax error I showed

paulmedynski · 2025-08-29T12:56:57Z

/azp run

azure-pipelines · 2025-08-29T12:57:17Z

Azure Pipelines successfully started running 2 pipeline(s).

benrr101 · 2025-09-24T16:59:04Z

This PR doesn't compile in its current state, and it has merge conflicts with the current main branch. Please let us know what you want to do with it in the next week otherwise we'll probably close this PR.

sharafabacery · 2025-09-28T22:46:04Z

@benrr101 close this issue the project in my machine doesn't compile also for that I will stop in this issue,I am sorry if I waste your time

fix dotnet#2971

453e915

sharafabacery requested a review from a team as a code owner July 26, 2025 10:08

benrr101 requested changes Jul 30, 2025

View reviewed changes

fix dotnet#2971

1d6ee75

sharafabacery requested a review from benrr101 July 30, 2025 19:53

Revert "fix dotnet#2971"

c5b7d30

This reverts commit 1d6ee75.

paulmedynski assigned benrr101, apoorvdeshmukh and paulmedynski Aug 29, 2025

benrr101 changed the title ~~fix #2971~~ Fix | Pass Empty List for SqlDataRecord (#2971) Sep 16, 2025

Fix | Pass Empty List for SqlDataRecord (#2971) #3518

Are you sure you want to change the base?

Fix | Pass Empty List for SqlDataRecord (#2971) #3518

Uh oh!

Conversation

sharafabacery commented Jul 26, 2025 • edited by benrr101 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues

Testing

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edwardneal commented Jul 30, 2025

Uh oh!

sharafabacery commented Jul 30, 2025

Uh oh!

edwardneal commented Aug 14, 2025

Uh oh!

sharafabacery commented Aug 25, 2025

Uh oh!

paulmedynski commented Aug 29, 2025

Uh oh!

azure-pipelines bot commented Aug 29, 2025

Uh oh!

benrr101 commented Sep 24, 2025

Uh oh!

sharafabacery commented Sep 28, 2025

Uh oh!

Uh oh!

sharafabacery commented Jul 26, 2025 •

edited by benrr101

Loading