-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
add support for guid column type in mssql and improve error logging #19016
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that it fixes GUID issue, user's complaint query was:
select t.[text] from sys.dm_exec_requests as r cross apply sys.dm_exec_sql_text(r.sql_handle) as t;
Now it fails because we don't support NTEXT type, should add implementation for that and see where it fails next.
Another bug discovered during testing: we cannot handle ORDER BY
statements and we should (not related to this PR)
@@ -229,6 +230,10 @@ def mssql_parse_tds_reply(data, info) | |||
|
|||
else | |||
col[:id] = :unknown | |||
|
|||
# See https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/ce3183a6-9d89-47e8-a02f-de5a1a1303de for details about column types | |||
info[:errors] << "Unsupported column type: #{col[:type]}. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should output col[:type]
as hex value, because that's what Microsoft uses in their reference doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
free tds uses integers, which is where I think the original values came from:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I don't mind hex or decimal output here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, then let's disregard this suggestion
Release NotesUpdates the MSSQL modules to support the GUID column type. Also improves error logging. |
This PR adds support for the
guid
datatype for TDS messaging as previously it would break our mssql message parsing.Additionally, it makes it more clear what datatype is not supported when running into an instance of an unsupported datatype.
Previously, an unsupported datatype would frequently result in a long string of
unsupported token
messages, or simply not return anything at all.Now, we should get specific
unsupported column type: <type identifier in decimal>
messages, and when we do getunsupported token
issues, there should only be one message accompanied by a list of functions run before getting there.To test guid types:
INSERT INTO guid_test (id) VALUES (NEWID());
Then do the following on both
master
and the current branch:query_interactive
shell or usingmssql_sql
module, run select queries against the database that should return values from the column you createdIn master, this should lead to a string of errors or empty output. In this pr, you should get a column with values that that looks like:
To test unknown column logging:
CREATE TABLE xml_table(Col1 int primary key, Col2 xml);
then on both master and the pr branch:
query_interactive
ormssql_sql
.New error: