-
Notifications
You must be signed in to change notification settings - Fork 910
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
Error with displaying results in sqlops - Query failed: Conversion overflows. #275
Error with displaying results in sqlops - Query failed: Conversion overflows. #275
Comments
@saurabh500 this looks at first glance to be a .net core issue. This works fine in SSMS (regular .Net Stack) but fails for us. Stack trace is below - it's reading a decimal into an object buffer using Could you help verify this is a dotnet issue rather than something we control, and if so get it tracked in corefx? Stacktrace is below, this is pretty easy to repro. Exception thrown: 'System.OverflowException' in System.Data.SqlClient.dll: 'Conversion overflows.'
at System.Data.SqlClient.SqlBuffer.get_Decimal()
at System.Data.SqlClient.SqlBuffer.get_Value()
at System.Data.SqlClient.SqlDataReader.GetValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData metaData)
at System.Data.SqlClient.SqlDataReader.GetValues(Object[] values)
at Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage.StorageDataReader.GetValues(Object[] values) in /Users/me/projects/sqltoolsservice/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/StorageDataReader.cs:line 116 |
@kevcunnane I see the same behavior where an exception is thrown on .Net Framework as well. The precision of upto 26 is supported both on .Net core and framework. using System;
using System.Data.Common;
using System.Data.SqlClient;
namespace SqlClientTest
{
class Program
{
static void Main(string[] args)
{
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder
{
DataSource = "localhost",
IntegratedSecurity = true
};
using (SqlConnection connection = new SqlConnection(builder.ToString()))
{
connection.Open();
DbCommand cmd = connection.CreateCommand();
cmd.CommandText = "SELECT CAST(80 AS DECIMAL(38, 27))"; // 38,26 works
DbDataReader reader = cmd.ExecuteReader();
while (reader.Read())
{
Object[] values = new Object[reader.FieldCount];
int result = reader.GetValues(values);
Console.WriteLine(values[0]);
}
}
}
}
} |
- Enable basic query execution by adding live connection handling to taskbar buttons - Remove database dropdown list - Add IConnectableEditor, IConnectableEditorParams, and INewConnectionParams - Add back IShowQueryResultsEditor
It looks like SSMS calls Given the impact of issue relative to the size of the change this is something we'll have to pick-up later when there's time to refactor the data reader. |
Thanks a bunch for looking into this and sending the update!
I may just be showing the level of my naivety, but for a little hack until
a better fix happens could we just wrap the GetValues in a try/catch and if
it catches this specific error try to get the value with GetSqlValues and
then round it to a supported length (if needed) and kick back into whatever
data type GetValues was returning (if different)?
Certainly a small issue, but if a simple little hack like that "fixes" it
for now, that'd be cool.
Thanks again!
…On Fri, Jul 27, 2018 at 9:05 PM Karl Burtram ***@***.***> wrote:
It looks like SSMS calls DbDataReader::GetSqlValues instead of
DbDataReader::GetValues. GetValues throws an System.Overflow exception
whereas GetSqlValues correctly returns the result. Unfortunately, simply
changing our reader to call GetSqlValues breaks a bunch of other stuff.
Given the impact of issue relative to the size of the change this is
something we'll have to pick-up later when there's time to refactor the
data reader.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#275 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Aglo4iZcKYAY0OVHmppc8e2Icyty9J_5ks5uK8begaJpZM4QzABX>
.
|
+1 on eilerth’s suggestion.
- Arvind.
________________________________
From: eilerth <[email protected]>
Sent: Sunday, July 29, 2018 7:16:55 PM
To: Microsoft/sqlopsstudio
Cc: ranasaria; Assign
Subject: Re: [Microsoft/sqlopsstudio] Error with displaying results in sqlops - Query failed: Conversion overflows. (#275)
Thanks a bunch for looking into this and sending the update!
I may just be showing the level of my naivety, but for a little hack until
a better fix happens could we just wrap the GetValues in a try/catch and if
it catches this specific error try to get the value with GetSqlValues and
then round it to a supported length (if needed) and kick back into whatever
data type GetValues was returning (if different)?
Certainly a small issue, but if a simple little hack like that "fixes" it
for now, that'd be cool.
Thanks again!
On Fri, Jul 27, 2018 at 9:05 PM Karl Burtram ***@***.***> wrote:
It looks like SSMS calls DbDataReader::GetSqlValues instead of
DbDataReader::GetValues. GetValues throws an System.Overflow exception
whereas GetSqlValues correctly returns the result. Unfortunately, simply
changing our reader to call GetSqlValues breaks a bunch of other stuff.
Given the impact of issue relative to the size of the change this is
something we'll have to pick-up later when there's time to refactor the
data reader.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#275 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Aglo4iZcKYAY0OVHmppc8e2Icyty9J_5ks5uK8begaJpZM4QzABX>
.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2Fsqlopsstudio%2Fissues%2F275%23issuecomment-408728173&data=02%7C01%7C%7Cc32546b750744b2ed5f608d5f5c285a6%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636685138163416092&sdata=3UcRhKgreiBdF5TfI9lqLlXdNO7jVYaTkKA3YPfQMIA%3D&reserved=0>, or mute the thread<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAnqWVh8nuNyWceul4_ewRDNS5fpfk3Kvks5uLmyXgaJpZM4QzABX&data=02%7C01%7C%7Cc32546b750744b2ed5f608d5f5c285a6%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636685138163416092&sdata=77EfiMCJLlZEJcZRhsVFqLdmlPG9drb5NcnPM1MDs2I%3D&reserved=0>.
|
FWIW this issue on the vscode-mssql project is probably the same thing: microsoft/vscode-mssql#1190 Cross-linking for visibility when people like me search, and in case it helps to bring these different application teams together! |
Slight advance on the impact of this bug. This trivial query works as expected:
However, running with the
returns
With the So the failure in the second column is also affecting the results in the first (and any/all other) columns of the result set. This seems like a possibly-preventable problem to me? Aside from @eilerth 's suggestion, can anything be done to prevent an error with one column affecting rendering of all the others? |
In my opinion I'd prefer not to have some of the values still displayed while the overflows are not, I'd rather it be obvious there is a problem with the rendered results so I don't mistakenly use the data that isn't complete. But I can see value in that suggestion, so thanks for commenting, @philosophicles . Also thanks for linking to the vscode issue for visibility. |
I can also understand that point of view :) |
Very small feature add to make sure that values entered for `TIME` columns are < 24hrs. If the value is >=24hrs when setting the column, an exception will be thrown that the edit/updateCell handler will catch and convert into an error on the JSON RPC side. * Adding validation of timespan columns * Fixing a merge conflict
the fix for the issue introduced an issue with money column type and needs to be reverted to unblock the Dec release. reopen the issue and will try to fix it in the next release. |
Hi! |
@leandroko could you please open a new issue with detailed descriptions? |
@alanrenmsft sure! thanks! |
Steps to Reproduce:
Execute this query: SELECT CAST(80 AS DECIMAL(38, 27))
Rather than returning a result value of 80, I get the error message "Query failed: Conversion overflows."
I can dump that value into a temp table with no problem:
SELECT CAST(80 AS DECIMAL(38, 27)) [a] INTO #a
And then if I try to query the temp table I get the same issue.
If I then cast as FLOAT it returns value just fine:
SELECT CAST([a] AS FLOAT) FROM #a
The text was updated successfully, but these errors were encountered: