Improve SQL Server type mapping documentation

[tangjiangling]

Description

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

SQL Server type mapping doc.

How would you describe this change to a non-technical end user or system administrator?

N/A.

Related issues, pull requests, and links

Fixes #11280
Related #11448

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

[tangjiangling]

Note: We mapped Trino CHAR/VARCHAR to SQL Server nchar/nvarchar in write operation, and the minimum length of SQL Server nchar/nvarchar is 1. But in the type mapping test we did not test what happens when we write CHAR(0)/VARCHAR(0) to SQL Server.

I'll send a pr to fix this.

[jhlodin]

Thanks for proposing these changes! Definitely an improvement. I commented some proposed changes that may be worth discussing, just want to find the best possible wording so we can start applying these changes to other connectors as well.

[tangjiangling]

@jhlodin Thanks for the review, it was very useful ~
And I will update this pr later today.

[tangjiangling]

Updated.
PTAL @jhlodin

[tangjiangling]

@ebyhr @hashhar Please also take a look to see if the mapping is correct.

[jhlodin]

Thanks @tangjiangling ! Would definitely like to see review from maintainers to see if we all agree on the new wording.

[ebyhr]

Could you remove types where we don't have tests? e.g. XML, uniqueidentifier and so on

[tangjiangling]

Could you remove types where we don't have tests? e.g. XML, uniqueidentifier and so on

Removed MONEY, SMALLMONEY, uniqueidentifier, XML, BINARY, IMAGE, TIMESTAMP, UDT, GEOMETRY, GEOGRAPHY.

If there is a strong demand from users one day, we can add it in.

[mosabua]

This is a great improvement. In the interest of making this a model for all other connectors I would like some input from @hashhar @electrum and others about better titles and descriptive text for the two mappings.

Here is my current (maybe faulty undestanding).

One mapping is for creating new columns in CTAS, ALTER TABLE, CREATE TABLE.

The other mapping is for working with existing columns. And that means for read (SELECT), but also write operations such as INSERT and UPDATE..

If this is correct .. what could be good titles for the two sections of mappings.

DB to Trino

and

Trino to DB

as we use them now are anything but intuitive ... we should come up with something better.. but what? Any suggestions?

[tangjiangling]

Ping, could anyone help to review it? (so I can move forward with fixing #11448)

[jhlodin]

Ping, could anyone help to review it? (so I can move forward with fixing #11448)

+1, would be great to use this as a jumping-off point for improving the rest of our connectors' type mapping sections.
@hashhar @electrum @ebyhr

[electrum]

Thanks! This is a big improvement.