Conversation
…evel 110
TRY_CONVERT only exists at database compatibility level 110+. On older
databases get_tables, get_columns and get_all_columns_batch failed with
error 195 ('nvarchar' is not a recognized built-in function name), which
broke opening a connection. Also aligns the get_triggers value() call with
the plain-literal form used elsewhere and adds live coverage for both.
egertaia
force-pushed
the
fix/triggers-xml-value-literal
branch
from
October 2, 2026 08:58
b4c940d to
d7e40cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported on Discord: opening a SQL Server connection failed with
Manual queries still worked.
Root cause
get_tables,get_columnsandget_all_columns_batchread extended-property comments withTRY_CONVERT(nvarchar(max), ep.value).TRY_CONVERTonly exists at database compatibility level 110 (SQL Server 2012) and above. At level 100 or lower, SQL Server doesn't treatTRY_CONVERTas a function at all. It parsesnvarchar(...)as a function call and fails with error 195. That's why the message namesnvarchar, notTRY_CONVERT.So the trigger is the database's compatibility level, not the server version. A database restored or upgraded from SQL Server 2008 keeps level 100 on a 2019 or 2022 server. Opening a connection runs these introspection queries, so the connection fails while ad-hoc queries work.
The first theory on Discord blamed the
N'...'literals in theget_triggersXMLvalue()call. That turned out to be wrong..value(N'.', N'nvarchar(max)')works on SQL Server 2017 and 2022 at every compatibility level I tried.Reproduction (SQL Server 2022 container)
At level 110 or higher the same statement returns
x.Fix
TRY_CONVERTis replaced withCONVERTin the three introspection queries.ep.valueis asql_variantholding the property text, so the conversion can't fail in practice.get_triggersvalue()call now uses plain literals ('.','nvarchar(max)'), matching the foreign-key queries inintrospection.rs. It wasn't the cause, but it keeps the code consistent.Tests
metadata_introspection_works_at_compatibility_level_100. It creates a database at level 100 and callsget_tables,get_columnsandget_all_columns_batch. Without the fix it fails with the exact Discord error. With the fix it passes.get_triggers_lists_events_and_timing.live_dbhad noget_triggerscoverage before.Verified locally against SQL Server 2022 (16.0.4295.3) in podman. The full
live_dbsuite passes (31/31), and so do the unit tests (219), clippy and fmt.Found while testing, not fixed here
At compatibility level below 130, the
REALcase inadvertised_types_round_trip_through_query_insert_update_and_nullfails. The update returnsSQL Server connection failure: IO error: Expected ColumnMetadata in context. There are two separate issues behind that:FLOATtoREALdifferently, so the boundary value3.4028235e38overflows (Msg 232). CI runs at level 160, so CI isn't affected.mssql-tds-preview'sdrain_stream(tds_client.rs) reads everything after anERRORtoken withParserContext::Noneand ignoresCOLMETADATA. So when one statement in a batch fails and a later statement returns rows, the crate can't parse those rows. This happens at every compatibility level.update_recordandinsert_recordalways hit it on runtime errors such as arithmetic overflow, because the plugin appendsSELECT @@ROWCOUNT. The user sees a misleading "connection failure" instead of the real SQL error.SELECT 1/0; SELECT 2throughexecute_queryalso hangs. The latestmssql-tds-preview(0.1.0-preview.9) has the samedrain_stream. This needs an upstream fix or a separate plugin-side workaround, so I'll handle it in its own issue or PR.