morningman commented on PR #67916:
URL: https://github.com/apache/doris/pull/67916#issuecomment-5654456698
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67916
commit: 39de21240f76b53ba3a5451c6a3f5070518205c3
base: 2573820600eb178150a35710a94fd58b478df96d
reviewed_at: 2026-09-14T00:15+08:00
reviewer: morningman
model: claude-opus-5
effort: max
findings: {blocker: 0, major: 0, minor: 2, nit: 2}
rounds: 1
converged: true
```
**Notes for maintainers**
-
`fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcSQLServerClient.java:128`
— the binary carve-out is wider than its reason: mssql-jdbc's `DataTypeFilter`
rewrites only ODBC `-151` (CLR UDT) to `VARBINARY`, so aliases over
`datetimeoffset` (`-155`), `binary` (`-2`) and `image` (`-4`) are unambiguous
and could mirror the name path (STRING / VARBINARY) instead of staying
UNSUPPORTED; `SELECT *` still fails for such tables. Same in
`JdbcSQLServerConnectorClient`. (Minor)
-
`docker/thirdparties/docker-compose/sqlserver/init/03-create-table.sql:291` — 7
fallback families (smallint, real, numeric, smallmoney, datetime,
smalldatetime, char/nchar/text/ntext, uniqueidentifier), `sysname`,
alias+IDENTITY and the UNSUPPORTED-alias case are unit-tested only; the
regression also exercises only the connector plugin path (fe-core
`JdbcSQLServerClient` is reached via streaming/CDC jobs alone). (Minor)
-
`fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcSQLServerClient.java:41`
— fe-core matches `TYPE_NAME` case-sensitively while the connector lowercases;
with the new explicit list a mixed-case alias such as `dbo.JSON FROM
nvarchar(max)` resolves to STRING in one and UNSUPPORTED in the other. (Nit)
-
`fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcSQLServerClient.java:159`
— decimal/datetime rules are duplicated between the two switches; fe-core
hard-codes `6` although `JdbcClient.JDBC_DATETIME_SCALE` exists. (Nit)
- Verified against mssql-jdbc 12.10.2 sources: CLR UDTs arrive as
`VARBINARY`, `sysname` is the only system type whose mapping changes, and the
BE scanner needs nothing because it reads by Doris type and TDS carries the
base type for alias columns.
<sub>Reviewed locally with the `doris-repo-review` pipeline. Repository
policy may accept this receipt for the matching commit; it is not a human
Apache approval.</sub>
<!-- doris-repo-review:v1:end -->
--
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]
