sandynz commented on PR #39516:
URL: https://github.com/apache/shardingsphere/pull/39516#issuecomment-5341529120
### Result
**Review Result: Mergeable**
No blocking issues were found. This commit only adds regression coverage
for empty-string primary keys, which the current production implementation
already supports correctly.
### Evidence
- `UniqueKeyIngestPositionTest` verifies that decoding `s,,foo_uuid`
preserves `""` instead of converting it to `null`.
- `InventoryTaskSplitterTest` uses a VARCHAR primary key with real
empty-string data and verifies the range `["", "foo_uuid"]`.
- `TextPrimaryKeyMigrationE2EIT` includes an empty-string primary key in a
real migration and runs both SMALL and LARGE consistency checks.
- The MySQL Native E2E processed 3,002 records, reached 100% inventory
completion, and returned `result=true` for both consistency checks.
- All 23 focused unit tests, 7 integration tests, and 1 E2E test passed.
### Coverage
- Reviewed head: `14722cb26acdb8c0210faa2aa56ab495eca5e63f`
- All 3 files changed by the commit were reviewed.
- Root-cause, blast-radius, tests/runtime, and convergence reviews were
completed.
- PostgreSQL and openGauss schemas permit empty-string primary keys, but
local runtime verification covered MySQL only.
- Existing local Proxy and E2E configuration changes and untracked files
were excluded from the review.
- This is a code-correctness result only; CI status was not reviewed.
--
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]
