System.Data.SqlClient clarify SqlDataReader get char

[Wraith2]

Sql Server doesn't have an individual char datatype and in desktop .net the SqlDataReader.GetChar is marked as not browsable and implemented as a NotSupportedException. The version in this repository lacks the browsable attribute (in src and ref) and I think it should be re-added.

The documentation for SqlDataReader.GetFieldValue<T> lists char as one of the permitted types but it will not work, there is no metadata support or way to get a char from the database. This means it will always throw an InvalidCastException and I believe it should throw NotSupportedException where T is char to align with GetChar. This would be a behaviour change and need adding to the documentation.

@saurabh500 @afsanehr @tarikulsabbir

[MarkPflug]

Sql Server doesn't have an individual char datatype

char, nchar? If you don't specify a length (or specify length 1), they'll be a single character. I never understood why this wasn't implemented/supported. I've used single char columns for enum-like values where I'd rather use a mnemonic character than a number provided by tinyint. Can't a fixed-length 1 char column be identified by the metadata and handled via GetChar/GetFieldValue<char>?

[Wraith2]

They're modelled as strings. I'm not proposing to change that.

We could also hide GetTimeSpan since no-one should really use it, it doesn't return a real timespan it interprets the rowversion as one which is confusing at best and incorrect at worst. Rowversion should probably be read as int64.

[MarkPflug]

They're modelled as strings. I'm not proposing to change that.

I'm not proposing to change that either, since it would be a breaking change to remove the ability to use GetString and GetChars for a char(1) column. But, shouldn't it be possible to also allow GetChar to be used in that specific case. In the case of a char(1) column, wouldn't GetString create a new string for each row? If, as in my case, you only had a small number of a enum values, it would be more efficient to read them as a .net char.

[Wraith2]

So far no-one has missed it. If you want to provide an implementation go ahead. I'm just asking about making it hidden as it is on desktop.

[AfsanehR-zz]

@Wraith2 the not browsable attributes can be taken into consideration. Will take this as enhancement, and could be up for grabs.

[Wraith2]

I realise there was an extra item i mentioned in the discussion and didn't edit into the original post

• add EditorBrowsableState.Never to SqlDataReader.GetChar and GetTimestamp in ref and src
• change GetFieldValue<T> to throw NotSupportedException where T : char

If that's approved I can do the work unless @MarkPflug wants to have a go.

[divega]

As recently announced in the .NET Blog, focus on new SqlClient features an improvements is moving to the new Microsoft.Data.SqlClient package. For this reason, we are moving this issue to the new repo at https://github.com/dotnet/SqlClient. We will still use https://github.com/dotnet/corefx to track issues on other providers like System.Data.Odbc and System.Data.OleDB, and general ADO.NET and .NET data access issues.

[initram]

It seems that part of this issue has been resolved already, as GetChar has already been marked as not editor browsable in #180 .
see https://github.com/dotnet/SqlClient/blame/master/src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs#L935
The changes to the exception behaviors, do not seem to have been accepted. So what part of this issue is "up for grabs"?
Does the attribute need to be added in other files?

[Wraith2]

If you've checked and I hid it but didn't close this issue then it's closable now.

[cheenamalhotra]

Thanks @Wraith2 for confirming, we'll close this thread then.