morningman commented on PR #67916:
URL: https://github.com/apache/doris/pull/67916#issuecomment-5662022181
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67916
commit: 80fe649113ebee7203507478875676caaa39fc86
base: 2573820600eb178150a35710a94fd58b478df96d
reviewed_at: 2026-09-14T17:40+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**
- The CI review bot's P1 is fixed and verified in both clients and end to
end (`test_alias_name`): delimited alias names are no longer cut to a system
type name. Note for the record: the `39de212` review dismissed "alias names
with spaces" as covered by the code fallback without checking the collision
with a system-type prefix; the collision pre-existed at the base.
-
`fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcSQLServerClient.java:55`
— F-03 (Nit): the new `identityBaseType` gate is outcome-neutral — for every
IDENTITY-eligible base the code fallback returns the same Doris type as the
name path, so dropping the truncation alone fixes the P1 identically;
`numeric(18, 0) identity` / `decimal(18,0) IDENTITY(1,1)` are not forms any
driver produces (`TYPE_NAME` is the server's `spt_datatype_info` string
verbatim: `int identity`, `decimal() identity`, `numeric() identity`).
-
`fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcSQLServerConnectorClient.java:97`
— F-02 (Minor, pre-existing): the connector lowercases `TYPE_NAME` while
fe-core does not; the PR body's "SQL Server rejects `CREATE TYPE dbo.[int]`"
was observed on a case-insensitive collation only — on a CS collation
`dbo.[INT] FROM varchar(10)` would map to INT here. System type names are
lowercase on both metadata paths, so dropping the lowercasing is the safe
direction.
-
`fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcSQLServerClient.java:211`
— F-01 (Minor, by design): aliases over `datetimeoffset`/`binary`/`image`
(unambiguous codes) stay UNSUPPORTED, now pinned by the negative fixture; with
the P1 fix an alias *named* like a binary type over that base (`[varbinary
alias] FROM varbinary`) loses the readability it had by truncation accident —
worth listing in the "Deliberately not resolved" paragraph or resolving via
`SS_UDT_ASSEMBLY_TYPE_NAME` / `SQL_DATA_TYPE`.
- The suite is tagged `p2,external` and the External Regression pipeline
sets `excludeGroups = "p1,p2"`, so CI never executes it; the author's local run
against SQL Server 2022 is the only execution — the PR body should say so
("nine blocks, run locally at `80fe649`"), and the release note should exclude
the five UNSUPPORTED families.
<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]
