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

SQLDataAdapter.FillSchema doesn't mark computed columns as readonly #275

Closed
Lohnegrim opened this issue Oct 18, 2019 · 13 comments · Fixed by #286
Closed

SQLDataAdapter.FillSchema doesn't mark computed columns as readonly #275

Lohnegrim opened this issue Oct 18, 2019 · 13 comments · Fixed by #286
Assignees

Comments

@Lohnegrim
Copy link

Lohnegrim commented Oct 18, 2019

Describe the bug

The Function SQLDataAdapter.FillSchema redurns the shema of a table, witch sets the ReadOnly Property of computed columns correcntly on the .Net Framework, but not in .Net Core.

To reproduce

Create a new VB.NET Project and add a reference to the NuGet Package 'Microsoft.Data.SqlClient'.
Add this Code to the Sub Main:

Dim SQLConBldr As SqlConnectionStringBuilder
        Dim sqlDataAdapter As SqlDataAdapter = New SqlDataAdapter
        Dim cb As SqlCommandBuilder = New SqlCommandBuilder
        Using SQLCon As New SqlConnection()
            SQLConBldr = New SqlConnectionStringBuilder
            SQLConBldr.DataSource = "Server"
            SQLConBldr.InitialCatalog = "DB"
            SQLConBldr.UserID = "User"
            SQLConBldr.Password = "PW"
            SQLConBldr.ApplicationName = "DataAdapterTest"
            SQLCon.ConnectionString = SQLConBldr.ConnectionString
            SQLConBldr.Clear()
            SQLConBldr = Nothing
            SQLCon.Open()
            Using c = SQLCon.CreateCommand
                c.CommandText = "Create table #t (ID int identity(1,1), sText varchar(12), sMemo as (convert(varchar,ID) + ' ' + sText))"
                c.ExecuteNonQuery()
            End Using
            sqlDataAdapter.SelectCommand = New SqlCommand("Select * from #t")
            sqlDataAdapter.SelectCommand.Connection = SQLCon
            cb.DataAdapter = sqlDataAdapter
            Using cmd = cb.GetInsertCommand
                Console.WriteLine(cmd.CommandText)
            End Using
            Using DT As New System.Data.DataTable
                sqlDataAdapter.FillSchema(DT, System.Data.SchemaType.Mapped)
                For Each DC As System.Data.DataColumn In DT.Columns
                    Console.WriteLine(DC.ColumnName & " AutoIncrement: " & DC.AutoIncrement & " ReadOnly: " & DC.ReadOnly)
                Next
            End Using
        End Using
SqlConnectionStringBuilder SQLConBldr;
            SqlDataAdapter sqlDataAdapter = new SqlDataAdapter();
            SqlCommandBuilder cb = new SqlCommandBuilder();
            using (SqlConnection SQLCon = new SqlConnection())
            {
                SQLConBldr = new SqlConnectionStringBuilder;
                SQLConBldr.UserID = "sa";
                SQLConBldr.ApplicationName = "DataAdapterTest";
                SQLCon.ConnectionString = SQLConBldr.ConnectionString;
                SQLConBldr.Clear();
                SQLConBldr = null;
                SQLCon.Open();
                using (var c = SQLCon.CreateCommand())
                {
                    c.CommandText = "Create table #t (ID int identity(1,1), sText varchar(12), sMemo as (convert(varchar,ID) + ' ' + sText))";
                    c.ExecuteNonQuery();
                }
                sqlDataAdapter.SelectCommand = new SqlCommand("Select * from #t");
                sqlDataAdapter.SelectCommand.Connection = SQLCon;
                cb.DataAdapter = sqlDataAdapter;
                using (var cmd = cb.GetInsertCommand())
                {
                    Console.WriteLine(cmd.CommandText);
                }

                using (DataTable DT = new DataTable())
                {
                    sqlDataAdapter.FillSchema(DT, System.Data.SchemaType.Mapped);
                    foreach (DataColumn DC in DT.Columns)
                    {
                        Console.WriteLine(DC.ColumnName + " AutoIncrement: " + DC.AutoIncrement.ToString() + " ReadOnly: " + DC.ReadOnly.ToString());
                    }
                }
            }

Expected behavior

Using the System.Data.SQLClient
The .Net Framework Returns the Line:

sMemo AutoIncrement: False ReadOnly: True

But on .Net Core it returns:

sMemo AutoIncrement: False ReadOnly: False

Using the Microsoft.Data.SQLClient
booth return:

sMemo AutoIncrement: False ReadOnly: False

Also only when using System.Data.SQLclient on the .Net Framework you will get this working SQLCommand for the Insert command:

INSERT INTO [#t] ([sText]) VALUES (@p1)

the Other tree compinations return:

INSERT INTO [#t] ([sText], [sMemo]) VALUES (@p1, @p2)

resulting it this Error if you try to use it:

SqlException (0x80131904): The column "sMemo" cannot be modified because it is either a computed column or is the result of a UNION operator.

to reporduce you have to add this Coe after the Line 'Console.WriteLine(cmd.CommandText);':

try
                    {
                        cmd.Parameters[0].Value = 'x';
                        if (cmd.Parameters.Count > 1)
                            cmd.Parameters[1].Value = System.DBNull.Value;
                        cmd.ExecuteNonQuery();
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine(ex.ToString());
                    }

Further technical details

Microsoft.Data.SqlClient version: 1.0.19269.1
.NET target: Framework 4.7.1 (Correct), Core 3.0.0 (Wrong)
SQL Server version: SQL Server 2017
Operating system: Windows 10 (1809)

Additional context
Add any other context about the problem here.

@yukiwongky yukiwongky self-assigned this Oct 18, 2019
@yukiwongky
Copy link
Contributor

@Lohnegrim I'm having trouble reproducing the expected behaviour is .NET Framework (I'm using 4.7.1). The ReadOnly property of sMemo is False even though I'm using .NET Framework. I tried with both Microsoft.Data.SqlClient versions 1.0.19269.1 and the latest one built from the current master branch, but both of them give me False. Is there anything missing from the repro code what I should be aware of? I noticed in the repro steps the InsertCommand is computed but never used in the code. Is it supposed to be used somewhere by sqlDataAdapter?

@Lohnegrim
Copy link
Author

Lohnegrim commented Oct 19, 2019

@v-kaywon

Is it supposed to be used somewhere by sqlDataAdapter?

Sorry My mistake, seems like Microsoft.Data.SqlClient always returns readonly: False, it only returnes true on System.Data.SqlClient, and on the .Net Framework.
I originally encountered this when trying to create a insert for a changed datattable and reduced it to this sample.
After that I checked with Microsoft.Data.SqlClient and seems to have mixed up the results.
So the Microsoft.Data.SqlClient.SQLDataAdapter.FillSchema always returns Readonly:False and because of that generates a Insert command witch tries to write to the computated column.

@saurabh500
Copy link
Contributor

This was a regression caused by PR dotnet/corefx#34393 which caused this issue.
The issue can be reproduced in System.Data.SqlClient 4.7.0 as well.

I have been able to narrow down to the change which caused the issue, but haven't looked into why the issue was caused.

@yukiwongky
Copy link
Contributor

yukiwongky commented Oct 24, 2019

The root cause comes from changing this line. col.ReadOnly was never set and was default to false, whereas col.updatability actually got the information from the column metadata packet (see here). I'm planning to revert this line and make a test for it.

@saurabh500
Copy link
Contributor

col.ReadOnly cannot be set, it is derived from col.Updatability and col.UpdatabilityUnknown.

@Wraith2 had changed the SqlMetaData to use flags. I feel there is a bug in the computation of IsReadOnly

@yukiwongky
Copy link
Contributor

@Lohnegrim Attached is the driver with the fix in PR #286. Can you try it out? Thanks!
Microsoft.Data.SqlClient.1.1.0-dev.zip

@Lohnegrim
Copy link
Author

@v-kaywon yes, this works.
Thanks.

@David-Engel David-Engel added this to the 1.1.0 milestone Oct 25, 2019
@David-Engel David-Engel modified the milestones: 1.1.0, 1.1.0-preview2 Nov 4, 2019
@David-Engel David-Engel changed the title SQLDataAdapter.FillSchema doesn't mark computed columns as readonly on .Net Core 3.0 SQLDataAdapter.FillSchema doesn't mark computed columns as readonly Nov 4, 2019
@BenKmann
Copy link

So i stumbled over this Problem right now.
It seems Microsoft.Data.SqlClient was fixed, but System.Data.SqlClient was not fixed.
System.Data.SqlClient in .NET Core now has a different and wrong behaviour to System.Data.SqlClient in .NET Framework.
Will this bug be fixed for System.Data.SqlClient in the future?

@Wraith2
Copy link
Contributor

Wraith2 commented Aug 10, 2022

Unlikely. Microsoft.Data.SqlClient supercedes System.Data.SqlClient. You should look into changing your project to use the new library.

@ErikEJ
Copy link
Contributor

ErikEJ commented Aug 10, 2022

@BenKmann what is preventing you from using Microsoft.Data.SqlClient ?

@BenKmann
Copy link

Most of our Base-Code-Libraries are for .NET Framework, which only use .NET Framework SQL-Client.
So only some Smaller-Projects use .NET "Core" and these fail now, because the System.Data.SqlClient can't detect Read-Only Columns, like Computed Columns.
So i hoped, because it is not an extension and just a fix, it would get patched to keep the SqlClient at least in "working" state.

But in the future we will Upgrade to Microsoft.Data.SqlClient. But this will take some time, to migrate even the .NET Framework Projects.

@BenKmann
Copy link

Ah ok it is broken since 4.7.0, so if i use System.Data.SqlClient 4.6.1 i have a working SqlClient.
So if someone else is finding this issue, just use System.Data.SqlClient 4.6.1. This one was working, before the 4.7. update broke it.

@roji
Copy link
Member

roji commented Aug 11, 2022

@BenKmann note that Microsoft.Data.SqlClient supports .NET Framework - you don't have to stay on System.Data.SqlClient just because a project targets .NET Framework.

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 a pull request may close this issue.

8 participants