Doris-Breakwater commented on issue #67793:
URL: https://github.com/apache/doris/issues/67793#issuecomment-5615974777

   Breakwater-GitHub-Analysis-Slot: slot_b4595a22787b
   
   ### Initial triage
   
   **Assessment: confirmed, actionable JDBC Catalog bug.** The report contains 
a minimal reproduction and the metadata values needed to establish the 
`varchar`-alias failure. No FE log or Doris profile is needed for that exact 
case. The issue currently has no labels; `kind/fix`, `kind/datasource`, and 
`area/catalog` would fit the scope.
   
   ### Verified facts
   
   1. In the 4.1.0 tag, 
[`JdbcClient.getJdbcColumnsInfo()`](https://github.com/apache/doris/blob/4.1.0/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java#L378-L408)
 calls `DatabaseMetaData.getColumns()` and builds a `JdbcFieldSchema` for every 
result row.
   2. 
[`JdbcFieldSchema(ResultSet)`](https://github.com/apache/doris/blob/4.1.0/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/util/JdbcFieldSchema.java#L64-L73)
 stores both `DATA_TYPE` and `TYPE_NAME` (the issue text's `setDataType(...)` 
wording is not the exact 4.1.0 implementation, but the data flow is equivalent).
   3. 
[`JdbcSQLServerClient.jdbcTypeToDoris()`](https://github.com/apache/doris/blob/4.1.0/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcSQLServerClient.java#L31-L97)
 dispatches only on the normalized `TYPE_NAME` and returns `Type.UNSUPPORTED` 
from the default branch. It does not consult `DATA_TYPE`.
   4. Therefore, with the reported metadata tuple `TYPE_NAME=customtexttype`, 
`DATA_TYPE=Types.VARCHAR`, `COLUMN_SIZE=50`, the result is deterministically 
`UNSUPPORTED`, and the later Nereids error is a consequence rather than the 
root cause.
   5. The latest `master` source still has the same default behavior, so this 
is not already fixed there.
   
   I did not independently run a SQL Server 2019 instance; the runtime metadata 
tuple is reporter-supplied. However, it is consistent with SQL Server 
alias-type semantics and the JDBC contract: `CREATE TYPE ... FROM ...` defines 
an alias over a native base type, while `DatabaseMetaData.getColumns()` exposes 
a JDBC `DATA_TYPE` separately from the database-dependent `TYPE_NAME` 
([Microsoft `CREATE TYPE` 
documentation](https://learn.microsoft.com/en-us/sql/t-sql/statements/create-type-transact-sql),
 [JDBC `DatabaseMetaData` 
documentation](https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/java/sql/DatabaseMetaData.html#getColumns(java.lang.String,java.lang.String,java.lang.String,java.lang.String))).
   
   ### Review of the proposed fix
   
   Using the JDBC type code only as a fallback from the name-based default 
branch is a reasonable direction: it preserves SQL Server-specific name 
mappings for ordinary columns and fixes the supplied `VARCHAR` alias case.
   
   The pasted patch should not be applied verbatim because it is based on a 
different/older implementation shape. In 4.1.0, the change must preserve:
   
   - `Optional` access for `dataTypeName`, `columnSize`, and `decimalDigits`;
   - identity/parameter stripping already performed before the switch;
   - `money -> DECIMAL(19,4)` and `smallmoney -> DECIMAL(10,4)`, not `DOUBLE`;
   - `createDecimalOrStringType()` for precision validation/fallback;
   - the reported datetime scale capped at 6, rather than forcing scale 6;
   - existing `image`/`binary`/`varbinary` behavior, including 
`enable.mapping.varbinary`.
   
   The shown fallback fixes the demonstrated string alias but does **not** yet 
justify the broader claim that all SQL Server alias types resolve like their 
base types. Legal alias bases also include `time`, binary types, 
`datetimeoffset`, `uniqueidentifier`, `money`, and others. Some are represented 
by driver-specific codes, and some codes are ambiguous: Microsoft documents 
`geometry`, `geography`, and CLR `udt` as JDBC `VARBINARY`, so blindly treating 
every unknown `TYPE_NAME` plus `Types.VARBINARY` as a binary alias could 
incorrectly make genuinely unsupported types queryable ([Microsoft JDBC type 
mappings](https://learn.microsoft.com/en-us/sql/connect/jdbc/using-basic-data-types)).
 Unknown CLR/special types must remain unsupported unless Doris has positively 
identified a non-assembly alias and its base type.
   
   ### Recommended next steps
   
   1. Accept a narrowly scoped PR for the safe JDBC-code fallback, starting 
with the demonstrated standard scalar mappings, while retaining the existing 
name-first switch and `Type.UNSUPPORTED` for unrecognized/vendor-specific codes.
   2. Add a focused FE test with `TYPE_NAME=customtexttype`, 
`DATA_TYPE=Types.VARCHAR`, and `COLUMN_SIZE=50`, asserting a Doris string type; 
include a control proving an unknown/CLR-like type still remains unsupported.
   3. Add an SQL Server external-catalog regression fixture using the issue's 
`CREATE TYPE`/`CREATE TABLE`, then assert both `DESC` and `SELECT *`. This 
validates schema discovery and the BE scanner, whose string path uses 
`ResultSet.getObject()` followed by string conversion.
   4. If the PR claims general alias-type support, first capture a metadata 
matrix from both reported driver versions for aliases over at least integer, 
decimal/money, date/datetime/time/datetimeoffset, character, uniqueidentifier, 
and binary bases. Include `DATA_TYPE`, `TYPE_NAME`, `COLUMN_SIZE`, 
`DECIMAL_DIGITS`, and any available source/UDT metadata, plus CLR/geometry 
controls. That evidence should determine whether a code-only fallback is 
sufficient or whether SQL Server-specific base-type resolution is required.
   
   No additional information blocks fixing the reported `varchar(50)` case. The 
broader alias-type scope needs the metadata matrix above; it should not be 
inferred from the single `VARCHAR` observation.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to