Add Synonyms to SqlServerConnector

[kirkhansen]

Description

Adds a configuration option to allow users to opt-in to having Trino be able to interact with SQL Server Synonym table types.
Fixes #24206

A lot of the implementation here was shared from the OracleConnector's version of handling synonyms.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x ) Release notes are required, with the following suggested text:

## SQL Server connector
 * Adds `sqlserver.synonyms.enabled` configuration option #24206

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[ebyhr]

Please add requireNonNull:

        requireNonNull(sqlExecutor, "sqlExecutor is null");
        requireNonNull(namePrefix, "namePrefix is null");
        requireNonNull(definition, "definition is null");

[ebyhr]

Rename to name(). https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#avoid-get-in-method-names-unless-an-object-must-be-a-java-bean

[ebyhr]

Let's add a new test class enabling sqlserver.synonyms.enabled config property and verify that the connector can access synonyms.

[ebyhr]

@kirkhansen Did you submit CLA?

[kirkhansen]

@kirkhansen Did you submit CLA?

@ebyhr I did submit that yesterday.

Also, upon further investigation, it looks to me (a very untrained eye in both Java and JDBC) that this may be a much bigger lift than anticipated. I tried testing this by building this jar and shoving it into a docker container with trino as the tests copied from oracle didn't seem to test the positive case of someone wanting to include synonyms. I also stood up an mssql docker container, created a synonym, then pointed trino via a catalog. Sadly, the synonym didn't show up. It appears that the metadata object may not contain SYNONYMS. I ran something like this inside of an overridden getTables within the sql connector.

DatabaseMetaData metadata = connection.getMetaData();

ResultSet rs = metadata.getTableTypes();
log.debug("Available Table Types:");
while (rs.next()) {
    log.debug(rs.getString("TABLE_TYPE"));
}

io.trino.plugin.sqlserver.SqlServerClient       Available Table Types:
2025-03-27T20:46:28.098Z        DEBUG   SplitRunner-20250327_204627_00000_n5dic.2.0.0-1-213     io.trino.plugin.sqlserver.SqlServerClient       SYSTEM TABLE
2025-03-27T20:46:28.098Z        DEBUG   SplitRunner-20250327_204627_00000_n5dic.2.0.0-1-213     io.trino.plugin.sqlserver.SqlServerClient       TABLE
2025-03-27T20:46:28.098Z        DEBUG   SplitRunner-20250327_204627_00000_n5dic.2.0.0-1-213     io.trino.plugin.sqlserver.SqlServerClient       VIEW

I believe this means the JDBC driver in use doesn't see SYNONYMS from MSSQL via common convention at all.
I think the oracle connector got away with this by having a connection property in the driver itself that causes the metadata to show up within the metadata.getTableTypes() call.

Since this getTables call returns a ResultSet, I'm not sure how to best proceed here. Even Microsoft's version uses sp_tables which doesn't support synonyms.

Any thoughts here on how this could get implemented would be great. I could see a combination of a SQL statement hitting sys.synonyms UNION ALL'd on a call to sp_tables, but that's a rather large change in how this metadata is collected currently.

I naively thought something like:

    @Override
    public ResultSet getTables(Connection connection, Optional<String> remoteSchemaName, Optional<String> remoteTableName)
            throws SQLException
    {
        // Unfortunately, the connection.getMetaData only returns table types of `SYSTEM TABLE`, `TABLE`, `VIEW`.
        // If synonyms are passed, we'll need to handle that logic ourselves here.
        DatabaseMetaData metadata = connection.getMetaData();

        // Fetch normal tables/views using JDBC metadata
        ResultSet standardTables = metadata.getTables(
                connection.getCatalog(),
                escapeObjectNameForMetadataQuery(remoteSchemaName, metadata.getSearchStringEscape()).orElse(null),
                escapeObjectNameForMetadataQuery(remoteTableName, metadata.getSearchStringEscape()).orElse(null),
                getTableTypes().map(types -> types.toArray(String[]::new)).orElse(null));

        // If synonyms are enabled, manually fetch from sys.synonyms
        if (synonymsEnabled) {
            String sql = "SELECT name AS TABLE_NAME, schema_name(schema_id) AS TABLE_SCHEM, 'SYNONYM' AS TABLE_TYPE FROM sys.synonyms";

            Statement stmt = connection.createStatement();
            ResultSet synonyms = stmt.executeQuery(sql);

            log.debug("Fetched synonyms from sys.synonyms");

            // Combine standard tables and synonyms into a single ResultSet
            return new CombinedResultSet(standardTables, synonyms);
        }

        return standardTables;
    }

but quickly realized we'd need to implement the full ResultSet interface in order to operate on two result sets which seems really silly.

[ebyhr]

I see it's a known issue of SQL Server's JDBC driver microsoft/mssql-jdbc#1814.

[kirkhansen]

Is there an appetite on your end for seeing an implementation that works by running system table queries manually here, or do you have other ideas you'd like to see tried?

[kirkhansen]

Perhaps the right way forward here is an attempt at a PR to the MSSQL driver to support this. I imagine the code would look really similar between what this would have to do and what the driver would be doing. If the MSSQL folks don't accept, it could get ported over here, if the Trino team would accept that kind of change here.

1. Add query to sys.synonyms as an option to mssql jdbc driver.
2. Update this MR to add the current modifications requested (could be done prior or at the same time as 1)
3. Wait a bit to see what mssql-jdbc driver maintainers have to say
4. Update this MR to use a new mssql-jdbc driver version if they accept, or update this MR to use the PR implementation if they reject.

[github-actions]

This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack.