Fix reflection error for SQL Server sysname type by registering it in the dialect

[NotCarlosSerrano]

Changes

Fixes SQL Server reflection issues when a database contains columns using the
sysname type.

sysname is a built-in SQL Server type (an alias for NVARCHAR(128)), but it
is not registered in SQLAlchemy's ischema_names for some dialect versions.
When sqlacodegen reflects a schema containing this type, the reflection process
fails because the type cannot be resolved.

This change registers sysname as NVARCHAR in the MSSQL dialect before
reflection:

engine.dialect.ischema_names.setdefault("sysname", NVARCHAR)

This allows sqlacodegen to correctly reflect databases that contain columns
defined with the sysname type.

Checklist

• You've added tests (in tests/) which would fail without your patch
• You've added a new changelog entry (in CHANGES.rst).

[coveralls]

coverage: 97.861%. remained the same
when pulling 7a12e7f on NotCarlosSerrano:fix/mssql-sysname-reflection
into 9234437 on agronholm:master.

[agronholm]

I'm not sure this is the right place for this fix. Shouldn't this be done against the actual dialect rather than patching downstream, here?

[NotCarlosSerrano]

Good point, thanks for pointing that out.

My understanding was that sysname is a built-in SQL Server alias (NVARCHAR(128)), but it isn't always present in ischema_names depending on the SQLAlchemy / dialect version. When sqlacodegen reflects a schema containing it, the reflection fails because the type cannot be resolved.

I added the registration here mainly as a defensive workaround so that reflection succeeds even when the underlying dialect does not include it. That said, I agree the proper place to fix this would be in the MSSQL dialect itself.

Would you prefer that this PR be dropped in favor of opening an issue/PR against SQLAlchemy instead? Alternatively, I could keep this as a small compatibility workaround here if supporting older dialect versions is desirable.

[agronholm]

Would you prefer that this PR be dropped in favor of opening an issue/PR against SQLAlchemy instead? Alternatively, I could keep this as a small compatibility workaround here if supporting older dialect versions is desirable.

It is preferable. Things would gradually get out of hand if we started accepting workarounds for all kinds of upstream bugs.

[NotCarlosSerrano]

Got it, that makes sense.

I'll close this PR and look into proposing the fix upstream in SQLAlchemy so it can be handled in the dialect itself.

Thanks for the review!